Repository navigation
fix(cli): make the generated temp app pass its own strict tsconfig, and gate it with real tsc (#3853) - #4113
Merged
Conversation
…nd gate it with real tsc (#3853) Both app generators write a tsconfig.json carrying strict, noUnusedLocals and noUnusedParameters, and nothing had ever run it: dev/serve/build all go through Vite, which transpiles without checking, and the config itself says noEmit. The strictness was declared and never enforced, and the generated sources had drifted 17 errors past it — the issue's table is 12 of them. Fixed at the templates: - dropped the five imports declared and never used (TS6133): `Link` in src/App.tsx, and `cn` / `Button` / `SidebarGroupContent` / `SidebarGroupLabel` in src/Layout.tsx; - typed DynamicIcon's and AppLayout's destructured props (TS7031). The five map-callback parameters (TS7006) follow by inference from the AppConfig / MenuItem shapes rather than needing annotations of their own, and making `className` optional retires the two TS2741 call sites the issue had not measured; - narrowed the Lucide namespace to the component-by-name shape the layout actually indexes, which retires the `@ts-expect-error` without adding `any`; - added src/vite-env.d.ts to BOTH generators — without `vite/client`'s ambient declarations the entry's `import './index.css'` has nothing behind it (TS2882), which is the one defect of the set that was never routed-specific. The gate: three real `tsc -p` runs over apps the generators really write (2.76s total, measured). Bare-specifier resolution failures are exempt, since nothing installs the temp app's dependencies here, but only after being checked against the manifest the generator itself writes — so the exemption cannot cover a typo or an undeclared import, and relative specifiers are never exempt. Paired with a self-test that plants the pre-fix templates back and pins the codes. The reachability gate gains one exemption, `.d.ts`: an ambient declaration is reached through the tsconfig's `include`, not the module graph. The tsc gate is what proves the exempted file carries its weight. The issue's 待核 note about `React.ReactNode` in src/theme-provider.tsx is NOT a defect: TypeScript allows a UMD global namespace in a TYPE position without allowUmdGlobalAccess (that flag governs value access). Verified against real @types/react — a bogus member of the same namespace reports TS2694, so the namespace really is resolving.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
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.
Fixes #3853
Both app generators in
packages/cli/src/utils/app-generator.tswrite atsconfig.jsoncarryingstrict,noUnusedLocalsandnoUnusedParameters, and until this PR nothing had ever run it —dev/serve/buildall go through Vite, which transpiles without type-checking, and the config itself saysnoEmit. Declared strictness, never enforced.Premise check (current
origin/main@877385a76)Reproduced by generating with the real entry points and running the workspace's
tsc -pwith the generatedtsconfig.json. The issue's table holds verbatim, and measuring found 17 errors rather than 12:Layout.tsx(117,50)childand(117,57)cIdxwere not measuredDynamicIconinvoked without theclassNameits inferred type made requiredimport './index.css'in the entry — and this one hits both generators, not just the routed one#4099changed template dep ranges since filing; as expected it moved none of these — measured, not assumed.The
theme-provider.tsxUMD note is 待核 → not reproducible, and no fix was madeReact.ReactNodewithout a React namespace import produces no diagnostic. TypeScript permits a UMD global namespace in a type position;allowUmdGlobalAccessgoverns value access, which this file never does. Verified that the namespace really resolves rather than silently degrading — a probe referencing a bogus member of the same namespace reportsTS2694: Namespace 'React' has no exported member …, and assigning a symbol toReact.ReactNodereportsTS2322. So the type is being checked, and it is correct. Reported rather than "fixed".The fix, at the template source
Linkinsrc/App.tsx(the only link in the app is insrc/Layout.tsx, which imports it itself), andcn/Button/SidebarGroupContent/SidebarGroupLabelinsrc/Layout.tsx;DynamicIcon's andAppLayout's destructured props. The five TS7006 callbacks then follow by inference from theAppConfig/MenuItemshapes rather than needing annotations of their own, and makingclassNameoptional retires the two TS2741 call sites;@ts-expect-erroras well. Noanywas added anywhere;src/vite-env.d.ts(/// reference types="vite/client") to both generators.viteis already in the generateddevDependencies, and this is the filecreate-vite's own React+TS template writes for exactly this reason.AppConfigis applied to a variable, not to a fresh object literal, so a userapp.jsoncarrying extra keys is unaffected by excess-property checking.The gate
Three real
tsc -pruns over apps the generators really write, inpackages/cli's own suite, spawning the workspace's TypeScript (createRequire(...).resolve('typescript/bin/tsc'), resolved from this package — never a global).Cost, measured: routed 1012ms + plain 770ms + self-test 981ms = 2.76s; the whole
app-generator.test.tsfile runs in 3.19s and thepackages/clisuite in ~22s.Module resolution. The temp app is generated under the repo root inside
.objectui-tmp/— which is whatcommands/dev.tsitself does ("always in cwd to keep node_modules access"), is already gitignored, and is what makesreact,react-dom,react-router-dom,@types/reactandvite/clientresolve from the rootnode_modulesthe way they do for the real command. This is the established mechanism, not a new linker: nothing is installed or symlinked. Underos.tmpdir()nothing resolves at all andReact.ReactNodedegrades intoCannot find namespace 'React'— a diagnostic about the test's setup wearing the costume of a defect in the generated source.What is exempt, and why it is not a hole.
@object-ui/*andlucide-reactgenuinely do not resolve here, because nothing installs the temp app's dependencies (thegeneration onto diskblock says why an install would prove nothing). Those are exempt — but only when the specifier is bare, and only after each one is checked against the manifest the generator itself writes. A relative specifier is never exempt, which is what keeps./index.css,./theme-providerand./Layoutunder the gate. That partition is the issue's own: its table is the errors "unrelated to whether@object-ui/*is installed".Reverse verification, direction predicted first. Reverting
app-generator.tstoorigin/mainand re-running turns both gates red with the full pre-fix inventory — TS6133 ×5, TS7031 ×4, TS7006 ×5, TS2741 ×2, TS2882 ×1 on the routed app, and TS2882 on the plain one. One diagnostic differs by environment and is worth naming: at repo root the pre-fix@ts-expect-errorreportsTS2578: Unused '@ts-expect-error' directive, because unresolvedlucide-reactmakes the indexed expressionanyand there is nothing left to suppress; withlucide-reactresolvable (probed by generating underpackages/components/) the directive is used instead and TS2578 does not appear. The fix removes the directive, so the gate is stable either way.The in-suite self-test plants the pre-fix templates back and pins the resulting codes. Its predicted set was right in 5 of 6 codes; the miss is recorded in the test rather than papered over: an unused
import typebinding reports TS6196 ("declared but never used"), not TS6133 ("its value is never read"), because a type-only binding is not a value.Reachability gate. It gains exactly one exemption,
.d.ts: an ambient declaration is pulled into the program by the tsconfig'sinclude, never by the module graph, so reachability-from-main.tsxwas the wrong measure for it. The exemption's width is pinned in both directions by a new test (src/vite-env.d.tsexempt, asrc/vite-env.tsorphan beside it still reported), and it opens no hole because the tsc gate goes red the moment that file stops carrying its weight.Verification
vitest run packages/cli/from the repo root: 101 passed (4 files), 22s.tsc --noEmitinpackages/cli: exit 0.eslint packages/cli/src: 0 errors (15 warnings, all pre-existing on untouched lines).node scripts/check-changeset-presence.mjsandcheck-changeset-no-major.mjs: both green.Changeset:
patchon@object-ui/cli— template source changes ship inside the released package, and the Bump Policy forbidsmajorin this fixed group.Out of scope
Filed as #4111:
commands/init.tsis a third generator with its owntsconfigobject, alsostrict+noUnusedLocalsand also lacking avite-env.d.ts, so itssrc/main.tsxhas the same./index.cssTS2882 (measured, not inferred). It is outside this card's two generators, so it is filed rather than fixed here — but it is more severe than the defect this PR fixes: that scaffold's generatedpackage.jsonrunsbuild: "tsc && vite build", so the error is live for any user who runsobjectui initfollowed bynpm run build.Generated by Claude Code