Combined critique #56: root base tsconfig, dedup compiler policy - #857
Merged
Merged
Conversation
Extract shared compiler policy (target/module/moduleResolution/strict/ skipLibCheck/isolatedModules/verbatimModuleSyntax/noEmit) into tsconfig.base.json, extended by the 3 leaf tsconfigs with no `paths` mapping (apps/web, examples/tanstack-host, packages/jgengine). Verified via isolated repro that Bun 1.3.11's test runner does not apply tsconfig `paths` through `extends` unless `baseUrl` is also set in the same file (oven-sh/bun#23695, open upstream bug) — and tsgo (this repo's mandated compiler) has removed `baseUrl` support entirely (TS5102), making the two requirements mutually exclusive. The other 26 leaf tsconfigs (every package/game/app with a `paths` map) therefore keep their duplicated compiler flags as-is rather than risk breaking `bun test` deep-import resolution. No drift reconciliation beyond the 3 deduped configs: all leaf configs already agreed on target/module/moduleResolution/strict/skipLibCheck/ isolatedModules (100% consistent). verbatimModuleSyntax was previously absent only from examples/next-host and examples/studios (esModuleInterop used instead); left untouched since they can't safely extend the base. noUnusedLocals/noUnusedParameters/noFallthroughCasesInSwitch were already package-specific opt-ins (10 Games + convex/react/shell/ next-host/studios lack them) — tested forcing these on everywhere and it broke 4 workspaces (unused-local errors), so left as pre-existing per-package strictness, not moved to base.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
New
tsconfig.base.jsonat repo root holding the shared compiler policy (target/module/moduleResolution/strict/skipLibCheck/isolatedModules/verbatimModuleSyntax/noEmit). Extended by the 3 leaf tsconfigs with nopathsmapping:apps/web,examples/tanstack-host,packages/jgengine.Why only 3 of 29 leaf configs
Verified with an isolated repro that Bun 1.3.11's test runner does not apply tsconfig
pathsthroughextendsunlessbaseUrlis also declared in the same file — a known, open upstream bug (oven-sh/bun#23695). Buttsgo(this repo's mandated compiler,@typescript/native-preview) has fully removedbaseUrlsupport (TS5102: Option 'baseUrl' has been removed), unconditionally, anywhere in the resolvedextendschain.Those two constraints are mutually exclusive for any file both
bun testandtsgoread. The other 26 leaf tsconfigs (every package/game/app that declares a@jgengine/*deep-importpathsmap) therefore keep their duplicated compiler flags exactly as before — extending the base there would silently breakbun testdeep-import resolution whilecheck-typesstays green (that's the trap).Drift reconciliation
All 29 leaf configs already agreed 100% on
target/module/moduleResolution/strict/skipLibCheck/isolatedModules— no actual conflicting values, just repetition.verbatimModuleSyntaxwas previously absent only fromexamples/next-hostandexamples/studios(they useesModuleInteropinstead) — left untouched since they can't safely extend the base.noUnusedLocals/noUnusedParameters/noFallthroughCasesInSwitchare pre-existing per-package opt-ins (10Games/*+convex/react/shell/next-host/studioslack them). Tested forcing these on everywhere via the base — it broke 4 workspaces withTS6133unused-local errors, so left as-is rather than loosen the gate or force an unrelated cleanup into this PR.Gate
bun install— cleanbun run build— cleanbun scripts/check-types-all.ts— 29/29 passing (baseline, unchanged)bun run test:all— 4503 pass / 0 fail (baseline, unchanged) — explicitly re-verified the two@jgengine/*deep-import test files that broke under a naive full extraction (apps/dev/src/demo/behaviourRegisterDemo.test.ts,Games/starhome/src/game/starhome.world.test.ts) now passNo stray files — diff is
tsconfig.base.json+ the 3 leaf tsconfigs only.Generated by Claude Code