fix(build): exclude config files from production DTS rollup#10358
fix(build): exclude config files from production DTS rollup#10358ma-cote wants to merge 6 commits intoTanStack:mainfrom
Conversation
The `experimentalDts` option in tsup respects the tsconfig `include`
array to determine which files are in scope for DTS generation. Each
package's `tsconfig.json` includes `*.config.*` for IDE support, and
`tsconfig.prod.json` inherited this without overriding it. This caused
`vite.config.ts` and `tsup.config.ts` to be swept into the DTS rollup,
leaking `import { UserConfig } from 'vite'` into published types. Since
vite references `@types/node`, this globally overrides the DOM
`setTimeout` return type from `number` to `NodeJS.Timeout` for all
consumers.
Add `"include": ["src"]` (and `"exclude": ["src/__tests__"]` where
applicable) to all `tsconfig.prod.json` files that were missing it,
matching the pattern already used by query-core, react-query,
preact-query, and solid-query.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughProduction TypeScript configs across many packages now explicitly include Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/react-query-next-experimental/tsconfig.prod.json (1)
8-8: Consider adding"exclude": ["src/__tests__"]for consistency.While this package doesn't currently have a
src/__tests__directory (making the omission functionally harmless), all other packages in this PR include both"include": ["src"]and"exclude": ["src/__tests__"]. Adding the exclude clause would maintain pattern consistency across the monorepo and future-proof the config if tests are added later.📋 Proposed change for consistency
}, - "include": ["src"] + "include": ["src"], + "exclude": ["src/__tests__"] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-query-next-experimental/tsconfig.prod.json` at line 8, The tsconfig.prod.json currently only has "include": ["src"]; add an "exclude": ["src/__tests__"] entry to match other packages and future-proof the config (update the JSON in tsconfig.prod.json so it contains both the existing "include" key and a new "exclude": ["src/__tests__"] key).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/react-query-next-experimental/tsconfig.prod.json`:
- Line 8: The tsconfig.prod.json currently only has "include": ["src"]; add an
"exclude": ["src/__tests__"] entry to match other packages and future-proof the
config (update the JSON in tsconfig.prod.json so it contains both the existing
"include" key and a new "exclude": ["src/__tests__"] key).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 99366368-c38c-4134-8e96-0a0556c89575
📒 Files selected for processing (16)
packages/angular-query-experimental/tsconfig.prod.jsonpackages/angular-query-persist-client/tsconfig.prod.jsonpackages/eslint-plugin-query/tsconfig.prod.jsonpackages/preact-query-devtools/tsconfig.prod.jsonpackages/preact-query-persist-client/tsconfig.prod.jsonpackages/query-async-storage-persister/tsconfig.prod.jsonpackages/query-broadcast-client-experimental/tsconfig.prod.jsonpackages/query-devtools/tsconfig.prod.jsonpackages/query-persist-client-core/tsconfig.prod.jsonpackages/query-sync-storage-persister/tsconfig.prod.jsonpackages/react-query-devtools/tsconfig.prod.jsonpackages/react-query-next-experimental/tsconfig.prod.jsonpackages/react-query-persist-client/tsconfig.prod.jsonpackages/solid-query-devtools/tsconfig.prod.jsonpackages/solid-query-persist-client/tsconfig.prod.jsonpackages/vue-query/tsconfig.prod.json
…iles Adds scripts/verify-dts-imports.ts that scans all published .d.ts files for forbidden imports from build tools (vite, tsup, vitest) and `/// <reference types="node" />`. Wired into test:pr and test:ci to run after the build completes.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/verify-dts-imports.ts`:
- Line 13: The script currently continues when the glob search (const files =
await glob([...]) in verify-dts-imports.ts) returns zero results, which yields a
misleading "Verified 0 .d.ts files..." message; update the code to treat zero
matches as a hard failure by checking if files.length === 0 after the glob call,
logging a clear error that no .d.ts/.d.cts files were found (include the glob
patterns or context), and exit the process with a non-zero status (or throw an
error) so CI fails when no declaration files are discovered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 50759ed7-b704-407c-91c4-0db08e3b6c74
📒 Files selected for processing (2)
package.jsonscripts/verify-dts-imports.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/verify-dts-imports.ts`:
- Around line 51-53: Update the success message in the console.log that
currently reads "Verified ${files.length} .d.ts files — no build tool type leaks
found." to accurately reflect the scanned file types (both .d.ts and .d.cts).
Modify the message in the same module where the console.log is emitted (keeping
the ${files.length} interpolation) to say something like "Verified
${files.length} declaration files (.d.ts and .d.cts) — no build tool type leaks
found." so it matches the scan behavior initiated earlier in the script.
- Around line 11-13: The success console message currently printed after running
the glob (see the files variable populated by
glob(['packages/*/build/**/*.d.ts', 'packages/*/build/**/*.d.cts'])) mentions
only .d.ts files; update that log (the success message near where files.length
is used) to accurately state both file types — e.g., "Verified ${files.length}
.d.ts and .d.cts files — no build tool type leaks found." — reference the files
variable and the glob call to locate the spot to change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4a08e432-b04f-44a6-ab34-3ade703df5e4
📒 Files selected for processing (1)
scripts/verify-dts-imports.ts
Closes #10294
Summary
"include": ["src"]and"exclude": ["src/__tests__"]to 16tsconfig.prod.jsonfiles that were missing itvite.config.tsandtsup.config.tsfrom being swept into theexperimentalDtsrollup@types/nodetype pollution that overrides DOM'ssetTimeoutreturn type fromnumbertoNodeJS.Timeoutfor all consumersRoot cause
Commit 67cf8b6 switched from
dts: truetoexperimentalDts: trueinscripts/getTsupConfig.js. The experimental DTS bundler respectstsconfig.json'sincludearray, which contains*.config.*for IDE support. Sincetsconfig.prod.jsondidn't overrideinclude, build config files leaked into the published types. 4 packages (query-core, react-query, preact-query, solid-query) already had the fix — this PR brings the remaining 16 in line.Test plan
@tanstack/react-query-devtools— confirmed_tsup-dts-rollup.d.tsno longer containsimport { UserConfig } from 'vite'orimport { Options } from 'tsup'Summary by CodeRabbit
Bug Fixes
Chores