fix(query-core): use built-in NoInfer for generic indexed-access types#10593
fix(query-core): use built-in NoInfer for generic indexed-access types#10593ousamabenyounes wants to merge 4 commits intoTanStack:mainfrom
Conversation
github-actions
Bot
added
package: react-query
package: vue-query
package: query-core
labels
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
💤 Files with no reviewable changes (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughRemoves a custom ChangesNoInfer removal and propagation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
View your CI Pipeline Execution ↗ for commit 60991bf
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/react-query/src/__tests__/useQuery.test-d.tsx (1)
382-382: Optional: assert the test scenario more directly.
expectTypeOf(Test).toBeFunction()is essentially a no-op that exists to keepTestreferenced. Since the regression you're guarding against is purely compile-time (the body ofTestfailing to type-check), theitblock compiling is the real assertion. You could either drop the assertion (the function's existence already guards the regression) or assert something more meaningful, e.g. that thedatainsideTest's body is assignable toDataTypeToEntity[TDataType] | undefined. Not blocking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react-query/src/__tests__/useQuery.test-d.tsx` at line 382, The current test only references Test via expectTypeOf(Test).toBeFunction(), which is a weak/no-op; replace or strengthen it by asserting the compile-time assignment you care about: inside the test suite replace the expectTypeOf(Test).toBeFunction() line with a type assertion that the local variable used in Test (named data) is assignable to DataTypeToEntity[TDataType] | undefined (or drop the assertion entirely and rely on the test body compiling), e.g. use TypeScript's expectType or similar utility to assert data is of type DataTypeToEntity[TDataType] | undefined so the regression is guarded by an explicit compile-time check of data's type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/react-query/src/__tests__/useQuery.test-d.tsx`:
- Around line 341-384: The new describe block describe('generic indexed access
TData', …) is incorrectly nested inside describe('initialData', …); move the
entire describe('generic indexed access TData', ...) block so it is a sibling of
describe('initialData') under the top-level describe('useQuery', …) (i.e., close
the initialData describe before this new describe). Concretely, relocate the
describe block (the entire chunk containing enum DataType, DataTypeToEntity,
getData, getLabel, function Test, and the expectTypeOf assertion) to be placed
after the closing brace of describe('initialData', …) so it sits at the same
level as initialData and not inside it.
---
Nitpick comments:
In `@packages/react-query/src/__tests__/useQuery.test-d.tsx`:
- Line 382: The current test only references Test via
expectTypeOf(Test).toBeFunction(), which is a weak/no-op; replace or strengthen
it by asserting the compile-time assignment you care about: inside the test
suite replace the expectTypeOf(Test).toBeFunction() line with a type assertion
that the local variable used in Test (named data) is assignable to
DataTypeToEntity[TDataType] | undefined (or drop the assertion entirely and rely
on the test body compiling), e.g. use TypeScript's expectType or similar utility
to assert data is of type DataTypeToEntity[TDataType] | undefined so the
regression is guarded by an explicit compile-time check of data's type.
🪄 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: 2268aa9f-4f19-4ae2-8111-03e4fb810e30
📒 Files selected for processing (5)
.changeset/query-core-noinfer-builtin.mdpackages/query-core/src/noInfer.tspackages/query-core/src/types.tspackages/react-query/src/__tests__/useQuery.test-d.tsxpackages/vue-query/src/queryClient.ts
|
Thanks @coderabbitai — addressed in d942386. Lifted |
|
Tip For best results, initiate chat on the files or code changes.
🧠 Learnings usedIf you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
|
Thanks @coderabbitai — addressed the optional nit in 24c74b9. Tried strengthening with both Per your suggestion ("the function's existence already guards the regression"), kept the implicit assertion ( Type tests + lint all green locally. |
|
Tip For best results, initiate chat on the files or code changes.
✏️ Learnings added
🧠 Learnings used |
|
@TkDodo done — dropped the public |
…ss types The local `NoInfer<T> = [T][T extends any ? 0 : never]` trick breaks down when `T` is a generic indexed-access type such as `DataTypeToEntity[DT]`, producing errors like "Type '[DataTypeToEntity[DT]]' is not assignable to type 'DataTypeToEntity'". Since the project already requires TypeScript >= 5.4, delegate the public `NoInfer<T>` alias to TypeScript's built-in `NoInfer` (introduced in 5.4). A small `noInfer.ts` helper exposes it as `IntrinsicNoInfer<T>` to avoid the local-shadow recursion problem. Vue's `setQueryData` implementation now forwards the `<TData>` generic to `super.setQueryData<TData>(...)` so its declared `NoInfer<TData> | undefined` return type stays satisfied under the stricter built-in `NoInfer`. Fixes TanStack#9937
…f 'initialData'
Block was nested inside `describe('initialData', ...)` but is unrelated
to initialData behavior. Lift it to sibling level under
`describe('useQuery', ...)` so test output groups it correctly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…exed-access NoInfer Per CodeRabbit nitpick: clarify that the `getLabel(props.dataType, data)` call itself is the regression guard. With the previous hand-rolled NoInfer, this call failed to type-check; with built-in NoInfer (TS 5.4+), it compiles. An explicit `expectTypeOf(data).toEqualTypeOf<...>()` / `toMatchTypeOf<...>()` cannot work here — vitest's type-relation checks reduce to `Extends<NoInfer<T>, T>` which TS leaves unresolved under a generic `TDataType extends DataType` constraint. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per @TkDodo's review: TypeScript's intrinsic NoInfer (TS ≥ 5.4) is the canonical type, and the package already requires TS ≥ 5.4. Removing the re-export sidesteps the self-shadowing issue entirely and avoids encouraging consumers to import a duplicate.
ousamabenyounes
force-pushed
the
fix/issue-9937
branch
from
23f2a6d to
60991bf
Compare
2 participants
Summary
Fixes #9937.
Root cause
The public
NoInfer<T>exported from@tanstack/query-coreis defined as the historical pre-TS-5.4 trick:When
Tis a generic indexed-access type (e.g.DataTypeToEntity[DT]inside a function generic overDT extends keyof DataTypeToEntity), TypeScript fails to recognize thatNoInfer<DataTypeToEntity[DT]>andDataTypeToEntity[DT]are the same:The package already requires TypeScript ≥ 5.4 (lowest CI target is
typescript@5.4), so the built-in intrinsicNoInferis available everywhere it ships.Fix
packages/query-core/src/noInfer.tsthat re-exports the built-inNoInferunder anIntrinsicNoInfer<T>alias, sidestepping the self-shadowing problem when delegating from a same-named export.export type NoInfer<T>inpackages/query-core/src/types.tstoIntrinsicNoInfer<T>.packages/vue-query/src/queryClient.ts:super.setQueryData<TData>(...)forwards theTDatageneric explicitly so the stricter built-inNoInfer<TData>return type stays satisfied (previously theunknownreturned from the unparameterized call was passing only because the loose tuple-index trick masked it).The public
NoInferexport keeps the same identity, so no consumer-visible API change.Verification
Added type test in
packages/react-query/src/__tests__/useQuery.test-d.tsx(generic indexed access TData) that mirrors the issue's repro:useQueryinside a function generic over<TDataType extends DataType>whose data is then forwarded togetLabel(_, data: DataTypeToEntity[TDataType]). Without the fix it errors with the exact[DataTypeToEntity[DT]]is not assignable message; with the fix it type-checks.Local check matrix (all green):
pnpm exec nx run @tanstack/query-core:test:libpnpm exec nx run @tanstack/query-core:test:types(TS 5.4–6.0)pnpm exec nx run @tanstack/query-core:test:eslintpnpm exec nx run @tanstack/react-query:test:libpnpm exec nx run @tanstack/react-query:test:types(TS 5.4–6.0)pnpm exec nx run @tanstack/react-query:test:eslintpnpm exec nx run @tanstack/vue-query:test:libpnpm exec nx run @tanstack/vue-query:test:types(TS 5.4–6.0)pnpm exec nx run @tanstack/vue-query:test:eslintpnpm exec nx run @tanstack/{preact,solid,svelte,angular-query-experimental,query-persist-client-core}:test:typesTest plan
TData should depend from only arguments, not the resultts-expect-error test (ensuresNoInferstill blocks reverse inference) keeps erroringpatchfor@tanstack/query-coreand@tanstack/vue-query)Generated by Ora Studio
Vibe coded by ousamabenyounes
Summary by CodeRabbit
Bug Fixes
Tests
Chores
Generated by Ora Studio (with Claude Code)
Vibe coded by Ben Younes Ousama