-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
chore: remove unnecessary type assertions #10028
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
chore: remove unnecessary type assertions #10028
Conversation
|
📝 WalkthroughWalkthroughRemoves explicit type casts ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/query-core/src/queryObserver.ts (1)
546-551: Use a null-check for#selectError(don’t rely on truthiness).
SinceTErroris generic, a falsy thrown value (e.g.''/0) would currently skip the error path.Proposed fix
- if (this.#selectError) { - error = this.#selectError + if (this.#selectError !== null) { + error = this.#selectError data = this.#selectResult errorUpdatedAt = Date.now() status = 'error' }
🧹 Nitpick comments (2)
packages/query-devtools/src/Devtools.tsx (2)
1958-1966: LGTM removing theQueryState<unknown, Error>cast insetState.
One thing to keep an eye on:fetchMetais being used as an escape hatch for__previousQueryOptionsviaas any; if you want to tighten this later, a dedicated side-channel (e.g.,WeakMap<Query, QueryOptions>) avoids mutatingfetchMeta’s shape.
2183-2202: LGTM removing theQueryState<unknown, Error>cast in the loading-pathsetState.
Same note as above:fetchMeta: { … } as anyis still an intentional escape hatch; consider a side-channel if you ever want to remove the remainingany.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/query-core/src/query.tspackages/query-core/src/queryObserver.tspackages/query-devtools/src/Devtools.tsx
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: oscartbeaumont
Repo: TanStack/query PR: 9564
File: packages/solid-query-devtools/src/production.tsx:2-3
Timestamp: 2025-08-19T03:18:18.303Z
Learning: In the solid-query-devtools package, the codebase uses a pattern of type-only default imports combined with typeof for component type annotations (e.g., `import type SolidQueryDevtoolsComp from './devtools'` followed by `typeof SolidQueryDevtoolsComp`). This pattern is consistently used across index.tsx and production.tsx files, and the maintainers prefer consistency over changing this approach.
Learnt from: sukvvon
Repo: TanStack/query PR: 9892
File: packages/solid-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx:331-335
Timestamp: 2025-11-22T09:06:05.219Z
Learning: In TanStack/query test files, when a queryFn contains side effects (e.g., setting flags for test verification), prefer async/await syntax for clarity; when there are no side effects, prefer the .then() pattern for conciseness.
📚 Learning: 2025-08-19T03:18:18.303Z
Learnt from: oscartbeaumont
Repo: TanStack/query PR: 9564
File: packages/solid-query-devtools/src/production.tsx:2-3
Timestamp: 2025-08-19T03:18:18.303Z
Learning: In the solid-query-devtools package, the codebase uses a pattern of type-only default imports combined with typeof for component type annotations (e.g., `import type SolidQueryDevtoolsComp from './devtools'` followed by `typeof SolidQueryDevtoolsComp`). This pattern is consistently used across index.tsx and production.tsx files, and the maintainers prefer consistency over changing this approach.
Applied to files:
packages/query-core/src/query.tspackages/query-core/src/queryObserver.tspackages/query-devtools/src/Devtools.tsx
📚 Learning: 2025-11-02T22:52:33.071Z
Learnt from: DogPawHat
Repo: TanStack/query PR: 9835
File: packages/query-core/src/__tests__/queryClient.test-d.tsx:242-256
Timestamp: 2025-11-02T22:52:33.071Z
Learning: In the TanStack Query codebase, the new `query` and `infiniteQuery` methods support the `select` option for data transformation, while the legacy `fetchQuery` and `fetchInfiniteQuery` methods do not support `select` and should reject it at the type level.
Applied to files:
packages/query-core/src/queryObserver.tspackages/query-devtools/src/Devtools.tsx
📚 Learning: 2025-09-02T17:57:33.184Z
Learnt from: TkDodo
Repo: TanStack/query PR: 9612
File: packages/query-async-storage-persister/src/asyncThrottle.ts:0-0
Timestamp: 2025-09-02T17:57:33.184Z
Learning: When importing from tanstack/query-core in other TanStack Query packages like query-async-storage-persister, a workspace dependency "tanstack/query-core": "workspace:*" needs to be added to the package.json.
Applied to files:
packages/query-devtools/src/Devtools.tsx
🔇 Additional comments (3)
packages/query-core/src/query.ts (2)
390-406: Comment typo fix is fine (retryer).
No behavior change; the guard still reads correctly.
736-740: Good removal of the unnecessary cast aroundinitialDataUpdatedAt()call.
Keeps intent clear and should still type-narrow correctly via thetypeof === 'function'check.packages/query-core/src/queryObserver.ts (1)
335-346: Passingthis.optionsintoQuery.fetchdirectly looks correct.
AssumingQueryObserverOptionsstructurally containsQueryOptions(it should), this keeps types simpler without changing runtime behavior.
🎯 Changes
While testing my changes to the @typescript-eslint/no-unnecessary-type-assertion lint rule on this repo, I found these unnecessary type assertions that can be removed without impacting runtime behaviour or triggering TS errors.
typescript-eslint/typescript-eslint#11789
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.