fix(tanstack-query): exactOptionalPropertyTypes#878
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughProcedure utils across query packages now include Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant ProcedureUtils
participant QueryClient
Caller->>ProcedureUtils: build options (includes input)
alt input === skipToken
ProcedureUtils-->>Caller: return { enabled: false, ...options }
else
ProcedureUtils-->>Caller: return { ...options } (no enabled property)
end
Caller->>QueryClient: call query/mutation with options
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
dosubot
Bot
added
the
size:M
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @unnoq, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request addresses a type compatibility issue related to TypeScript's "exactOptionalPropertyTypes" compiler option within the TanStack Query ecosystem. It refines how the "enabled" property is handled in query options, ensuring that it is only present when explicitly set to "false" (e.g., when using "skipToken"). This change prevents the property from being implicitly set to "undefined", which can cause type errors or unexpected behavior when "exactOptionalPropertyTypes" is enabled.
Highlights
- Type Definition Refinement: The "enabled" property in "QueryOptionsBase" and "InfiniteOptionsBase" interfaces across various packages (React Query, Solid Query, Svelte Query, TanStack Query) has been updated from "boolean | undefined" to "?boolean", making it a truly optional property.
- Conditional Property Inclusion: The runtime logic for constructing query options has been modified to conditionally include the "enabled: false" property only when "skipToken" is used as input. This ensures that the "enabled" property is entirely omitted when it's not explicitly "false", aligning with the updated type definitions and "exactOptionalPropertyTypes" strictness.
- Enhanced Test Coverage: New test assertions have been added to verify that the "enabled" property is correctly omitted from query options when it's not explicitly set, confirming the intended behavior and type safety.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request correctly addresses an issue related to TypeScript's exactOptionalPropertyTypes compiler option. The changes are consistently applied across all affected packages (react-query, solid-query, svelte-query, and tanstack-query), ensuring that optional properties are omitted rather than being assigned undefined.
The main changes include:
- Updating type definitions from
enabled: boolean | undefinedtoenabled?: boolean. - Replacing explicit
undefinedassignment with a conditional object spread (...condition ? { prop: value } : {}) to omit the property when not needed. - Updating tests to assert the absence of the property using
'enabled' in options.
The implementation is clean, correct, and thorough. I have no further suggestions. Great work!
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (13)
packages/tanstack-query/src/procedure-utils.test.ts (4)
49-51: Prefer own-property check over "in" and consider dropping the redundant second assertionUsing the "in" operator can match properties up the prototype chain. For stronger intent, assert against own props; the follow-up undefined check is then redundant.
- expect('enabled' in options).toBe(false) - expect(options.enabled).toBeUndefined() + expect(Object.prototype.hasOwnProperty.call(options, 'enabled')).toBe(false)
101-103: Same as above: assert own-property absence for clarityAlign with the suggestion above to avoid prototype-chain surprises and reduce redundancy.
- expect('enabled' in options).toBe(false) - expect(options.enabled).toBeUndefined() + expect(Object.prototype.hasOwnProperty.call(options, 'enabled')).toBe(false)
187-189: Same as above: use an own-property check for absenceKeeps tests unambiguous and consistent with exactOptionalPropertyTypes intent.
- expect('enabled' in options).toBe(false) - expect(options.enabled).toBeUndefined() + expect(Object.prototype.hasOwnProperty.call(options, 'enabled')).toBe(false)
274-276: Same as above: own-property absence and avoid redundant undefined assertionSmall consistency/readability win across the test suite.
- expect('enabled' in options).toBe(false) - expect(options.enabled).toBeUndefined() + expect(Object.prototype.hasOwnProperty.call(options, 'enabled')).toBe(false)packages/svelte-query/src/procedure-utils.ts (3)
80-82: SkipToken should force-disable regardless of consumer overrides; reorder spreads (and parenthesize for clarity)As written, a consumer could pass
enabled: trueand override the intendedenabled: falseforskipToken, causing the query to run and then throw fromqueryFn. If the intent is to always disable whenskipTokenis used, move the conditional spread after...optionsInso it wins.- ...optionsIn.input === skipToken ? { enabled: false } : {}, - ...optionsIn, + ...optionsIn, + ...(optionsIn.input === skipToken ? { enabled: false } : {}),If allowing consumer override is intentional, keep the current order but consider adding parentheses for readability:
- ...optionsIn.input === skipToken ? { enabled: false } : {}, + ...(optionsIn.input === skipToken ? { enabled: false } : {}),
104-106: Apply the same spread-ordering decision here for streamed optionsMirror the decision from
.queryOptionssoskipTokenconsistently forces disabled (or consistently allows override) across builders.- ...optionsIn.input === skipToken ? { enabled: false } : {}, - ...optionsIn, + ...optionsIn, + ...(optionsIn.input === skipToken ? { enabled: false } : {}),
122-124: Apply the same spread-ordering decision here for infinite optionsEnsure consistent behavior for
skipTokenacross all option builders.- ...optionsIn.input === skipToken ? { enabled: false } : {}, - ...(optionsIn as any), + ...(optionsIn as any), + ...(optionsIn.input === skipToken ? { enabled: false } : {}),packages/svelte-query/src/procedure-utils.test.tsx (3)
36-37: Use own-property check to assert absence; optionally switch to toBeUndefined for consistencyAvoid prototype-chain surprises and keep assertions consistent across packages.
- expect('enabled' in options).toBe(false) - expect(options.enabled).toBe(undefined) + expect(Object.prototype.hasOwnProperty.call(options, 'enabled')).toBe(false) + expect(options.enabled).toBeUndefined()
76-77: Same as above: prefer own-property check and consistent undefined assertion- expect('enabled' in options).toBe(false) - expect(options.enabled).toBe(undefined) + expect(Object.prototype.hasOwnProperty.call(options, 'enabled')).toBe(false) + expect(options.enabled).toBeUndefined()
135-136: Same as above: own-property absence and unified undefined assertion- expect('enabled' in options).toBe(false) - expect(options.enabled).toBe(undefined) + expect(Object.prototype.hasOwnProperty.call(options, 'enabled')).toBe(false) + expect(options.enabled).toBeUndefined()packages/react-query/src/procedure-utils.test.ts (3)
36-37: Prefer own-property absence check; unify undefined assertionUse an own-property check and align the undefined assertion with other packages for consistency.
- expect('enabled' in options).toBe(false) - expect(options.enabled).toBe(undefined) + expect(Object.prototype.hasOwnProperty.call(options, 'enabled')).toBe(false) + expect(options.enabled).toBeUndefined()
76-77: Same as above: own-property check and unified undefined assertion- expect('enabled' in options).toBe(false) - expect(options.enabled).toBe(undefined) + expect(Object.prototype.hasOwnProperty.call(options, 'enabled')).toBe(false) + expect(options.enabled).toBeUndefined()
135-136: Same as above: own-property absence and unified undefined assertion- expect('enabled' in options).toBe(false) - expect(options.enabled).toBe(undefined) + expect(Object.prototype.hasOwnProperty.call(options, 'enabled')).toBe(false) + expect(options.enabled).toBeUndefined()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
packages/react-query/src/procedure-utils.test.ts(3 hunks)packages/react-query/src/procedure-utils.ts(3 hunks)packages/react-query/src/types.ts(2 hunks)packages/solid-query/src/procedure-utils.test.ts(3 hunks)packages/solid-query/src/procedure-utils.ts(3 hunks)packages/solid-query/src/types.ts(2 hunks)packages/svelte-query/src/procedure-utils.test.tsx(3 hunks)packages/svelte-query/src/procedure-utils.ts(3 hunks)packages/svelte-query/src/types.ts(2 hunks)packages/tanstack-query/src/procedure-utils.test-d.ts(4 hunks)packages/tanstack-query/src/procedure-utils.test.ts(4 hunks)packages/tanstack-query/src/procedure-utils.ts(4 hunks)packages/tanstack-query/src/types.ts(2 hunks)
🔇 Additional comments (15)
packages/react-query/src/types.ts (2)
15-16: Makeenabledoptional: LGTMThis aligns with exactOptionalPropertyTypes and ensures the property can be omitted cleanly when not needed.
39-40: Infiniteenabledoptional: LGTMConsistent with the query options shape and the PR intent to omit
enabledunless necessary.packages/solid-query/src/types.ts (2)
23-24: Optionalenabledin QueryOptionsBase: LGTMMatches the intended behavior with exactOptionalPropertyTypes.
47-48: Optionalenabledin InfiniteOptionsBase: LGTMConsistent with other packages and procedure-utils behavior.
packages/svelte-query/src/types.ts (2)
23-24: Optionalenabledin QueryOptionsBase: LGTMCorrect use of optional property to avoid emitting
enabledunnecessarily.
47-48: Optionalenabledin InfiniteOptionsBase: LGTMConsistent with the rest of the changes in this PR.
packages/tanstack-query/src/types.ts (2)
53-54: Optionalenabledin QueryOptionsBase: LGTMMatches procedure-utils behavior to omit
enabledwhen not usingskipToken.
78-79: Optionalenabledin InfiniteOptionsBase: LGTMGood consistency across all packages.
packages/tanstack-query/src/procedure-utils.test-d.ts (4)
151-152: Types aligned with exactOptionalPropertyTypes: enabled is optionalChanging to
enabled?: booleanmatches the intent to omit the property when not usingskipToken. Looks good.
284-285: LGTM: streamed options enabled now optionalConsistent with the runtime behavior and tsconfig exact optional semantics.
414-415: LGTM: live options enabled now optionalMatches repo-wide change and prevents “present-but-undefined” pitfalls.
664-665: LGTM: infinite options enabled now optionalConsistent across all builders; no further action needed here.
packages/solid-query/src/procedure-utils.test.ts (3)
36-38: LGTM: Asserting absence of 'enabled' is correct with exactOptionalPropertyTypesThe new check using the in-operator precisely validates that 'enabled' is omitted when not using skipToken. The follow-up undefined check is fine as an additional guard.
76-78: LGTM: Streamed options also correctly omit 'enabled' when not using skipTokenThe assertion aligns with the new semantics.
135-137: LGTM: Infinite options omit 'enabled' when not using skipTokenConsistent with the runtime behavior and types.
More templates
@orpc/arktype
@orpc/client
@orpc/contract
@orpc/experimental-durable-event-iterator
@orpc/hey-api
@orpc/interop
@orpc/json-schema
@orpc/nest
@orpc/openapi
@orpc/openapi-client
@orpc/otel
@orpc/react
@orpc/react-query
@orpc/server
@orpc/shared
@orpc/solid-query
@orpc/standard-server
@orpc/standard-server-aws-lambda
@orpc/standard-server-fetch
@orpc/standard-server-node
@orpc/standard-server-peer
@orpc/svelte-query
@orpc/tanstack-query
@orpc/trpc
@orpc/valibot
@orpc/vue-colada
@orpc/vue-query
@orpc/zod
commit: |
1 participant
When using
"exactOptionalPropertyTypes": truein my tsconfig, I get an error saying thatenabled cannot be undefined (as it is marked explicitly as undefined by ORPC, but is optional for tanstack query):Summary by CodeRabbit
enabledis now optional (enabled?: boolean) in query and infinite option interfaces, improving TypeScript ergonomics without changing runtime behavior.