Skip to content

feat(evals): consolidate tsconfig setup and typecheck every eval in its own isolated TS program - #389

Open
mikolajadamowicz wants to merge 1 commit into
callstackincubator:mainfrom
mikolajadamowicz:tsconig
Open

mikolajadamowicz wants to merge 1 commit into
callstackincubator:mainfrom
mikolajadamowicz:tsconig

Conversation

@mikolajadamowicz

Copy link
Copy Markdown

Why

bun run typecheck only ever checked runner/. The root tsconfig.json listed evals/ in include, but also excluded it, so every eval fixture (app/, example/, reference/) had no typecheck gate at all, and CI never caught a broken eval.

The obvious fix, one shared tsc program over all of evals/, doesn't work. Several evals declare global type augmentations (react-navigation's declare global { namespace ReactNavigation { interface RootParamList ... } } } is the common one). Compiling every eval into the same program merges those interfaces across unrelated evals and produces false conflicts, most visibly across 17 files in navigation/*, where each eval extends RootParamList with a different, incompatible shape.

What changed

  • scripts/typecheck-evals.ts: new script that gives every eval directory its own isolated TypeScript program (via the compiler API), so global augmentations from one eval never leak into another. All programs share one compiler host, so common files like React Native's types and lib.d.ts are parsed once instead of once per eval. Run with bun run typecheck:evals.
  • evals/tsconfig.json: now the single source of eval compiler options, extended by nothing else. Also carries the one real exception this repo needs: expo-router/01-rn-expo-router-sdk56-react-navigation-imports/app/_layout.tsx intentionally imports legacy @react-navigation/native theming (that's the eval's premise), which conflicts with expo-router's vendored theming types when compiled alongside that eval's own reference/. The file is excluded with a comment explaining why.
  • Removed evals/tsconfig.expo.json and the typecheck:expo-evals script. They existed to give the three Expo categories a shared-program check that dodged the same collision problem for one file. typecheck:evals already covers those same evals per-fixture, without the collision risk, so the separate config was redundant.
  • tsconfig.json: added scripts/**/*.ts to include so the new script is itself covered by bun run typecheck.
  • Fixed a real bug this surfaced: evals/async-state/12-rn-jotai-atomwithstorage-async-rehydrate-guard/app/App.tsx read previous.theme directly inside atomWithStorage's setter, but previous there is typed Preferences | Promise<Preferences>. Now awaits it first, matching the pattern the eval's own reference/ already uses.

Known gap

25 skia evals still fail typecheck:evals, all with the same error: @shopify/react-native-skia isn't installed anywhere in the repo, so its types can't resolve.

This depends on #388 (#388), which adds @shopify/react-native-skia as a dependency and fixes four real bugs in skia reference solutions that the missing dependency had been hiding. That PR needs to merge first, or typecheck:evals keeps failing on all 25 skia evals here.

#388 was built against the old single-shared-program evals/tsconfig.json (it verifies with bun tsc -p evals/tsconfig.json --noEmit), before this PR's include/exclude changes and the move to per-eval isolation. Whichever PR merges second will likely need a small rebase on evals/tsconfig.json to reconcile the two.

Testing

  • bun run typecheck:evals: 109/134 evals pass. The 25 failures are exactly the skia evals above, nothing else.
  • bun run typecheck: clean.
  • bun run lint: clean.

@mikolajadamowicz

Copy link
Copy Markdown
Author

typecheck:evals is not wired into CI yet, matching how typecheck:expo-evals was left before this change. Wiring it in is a separate decision that should be made by maintainer i guess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant