Skip to content
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

refactor(*): improved type inference for useQueries with skipToken #7484

Merged
merged 15 commits into from
Jun 4, 2024

Conversation

gwansikk
Copy link
Contributor

@gwansikk gwansikk commented May 27, 2024

Summary

This PR includes three type inference improvements.

1️⃣ Accurate Type Inference When SkipToken is Present

This issue was previously fixed (#7150), but it has reoccurred due to various modifications. Upon investigation, it appears that the root cause of the issue is the unique symbol of the skipToken. A unique symbol represents a single specific symbol value, and TypeScript checks this very strictly. When a queryFn that includes a skipToken is provided in a readonly or const state (immutable literal value), type inference works correctly. However, in general, when declared with a skipToken, type inference does not work.

Improved Code ✅ Existing Code ❌
UseQueryOptionsForUseQueries Pasted Graphic 6

The current solution involves type widening for the symbol, which resolves the issue. With this approach, there is no longer a need for ReadonlyArray (as const). However, this might not work correctly in a JavaScript environment. Through type widening, a symbol other than SkipToken’s symbol might behave similarly. (In a TypeScript environment, this would be detected as a type error early on.) What do you think about this approach? I would like to hear your opinions.

// Widen the type of the symbol to enable type inference even if skipToken is not immutable.
type SkipTokenForUseQueries = symbol

2️⃣ The type inference process for GetUseQueryOptionsForUseQueries has been simplified, yet it performs more accurate type inference.

Before After
Pasted Graphic 3 Pasted Graphic 7

3️⃣ The type inference process for GetUseQueryResult has been simplified.

Before After
image image

Copy link

vercel bot commented May 27, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
query ⬜️ Ignored (Inspect) Visit Preview Jun 4, 2024 7:43am

Copy link

nx-cloud bot commented May 27, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit f3ebf03. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 2 targets

Sent with 💌 from NxCloud.

Copy link

codesandbox-ci bot commented May 27, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit f3ebf03:

Sandbox Source
@tanstack/query-example-angular-basic Configuration
@tanstack/query-example-react-basic-typescript Configuration
@tanstack/query-example-solid-basic-typescript Configuration
@tanstack/query-example-svelte-basic Configuration
@tanstack/query-example-vue-basic Configuration

Copy link

codecov bot commented May 27, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 78.15%. Comparing base (96aa461) to head (9c494a1).

Current head 9c494a1 differs from pull request most recent head f3ebf03

Please upload reports for the commit f3ebf03 to get more accurate results.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #7484       +/-   ##
===========================================
+ Coverage   43.79%   78.15%   +34.35%     
===========================================
  Files         183       90       -93     
  Lines        7021     1474     -5547     
  Branches     1535      319     -1216     
===========================================
- Hits         3075     1152     -1923     
+ Misses       3580      267     -3313     
+ Partials      366       55      -311     
Components Coverage Δ
@tanstack/angular-query-devtools-experimental ∅ <ø> (∅)
@tanstack/angular-query-experimental 86.58% <0.00%> (ø)
@tanstack/eslint-plugin-query ∅ <ø> (∅)
@tanstack/query-async-storage-persister ∅ <ø> (∅)
@tanstack/query-broadcast-client-experimental ∅ <ø> (∅)
@tanstack/query-codemods ∅ <ø> (∅)
@tanstack/query-core ∅ <ø> (∅)
@tanstack/query-devtools ∅ <ø> (∅)
@tanstack/query-persist-client-core ∅ <ø> (∅)
@tanstack/query-sync-storage-persister ∅ <ø> (∅)
@tanstack/react-query 92.72% <ø> (ø)
@tanstack/react-query-devtools 10.71% <ø> (ø)
@tanstack/react-query-next-experimental ∅ <ø> (∅)
@tanstack/react-query-persist-client 100.00% <ø> (ø)
@tanstack/solid-query 77.92% <100.00%> (ø)
@tanstack/solid-query-devtools ∅ <ø> (∅)
@tanstack/solid-query-persist-client 100.00% <ø> (ø)
@tanstack/svelte-query 67.33% <ø> (ø)
@tanstack/svelte-query-devtools ∅ <ø> (∅)
@tanstack/svelte-query-persist-client 100.00% <ø> (ø)
@tanstack/vue-query 70.91% <ø> (+0.44%) ⬆️
@tanstack/vue-query-devtools ∅ <ø> (∅)

@TkDodo
Copy link
Collaborator

TkDodo commented May 31, 2024

thanks. does this fix this issue, too?

if so, can you please add another type test for it?

gwansikk and others added 6 commits June 1, 2024 23:13

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…Token
@gwansikk gwansikk changed the title refactor(react-query): improved type inference for useQueries refactor(*): improved type inference for useQueries with skipToken Jun 1, 2024
@gwansikk
Copy link
Contributor Author

gwansikk commented Jun 1, 2024

Yes, I’ve confirmed that it also resolves issue #7496.

• This improvement has been applied to other adapters (Angular, Solid, Svelte, Vue) as well.
• A type check test has been added to ensure proper type inference.

@TkDodo
Copy link
Collaborator

TkDodo commented Jun 1, 2024

some of the useQueries tests in vue are failing now:

[vue-demi] Switched for Vue 3 (entry: "vue")
src/__tests__/useQueries.types.test.ts(141,13): error TS2322: Type 'true' is not assignable to type 'false'.
src/__tests__/useQueries.types.test.ts(142,9): error TS2344: Type 'false' does not satisfy the constraint 'true'.

@gwansikk
Copy link
Contributor Author

gwansikk commented Jun 4, 2024

All done! Please check for conflicts. @TkDodo

@TkDodo
Copy link
Collaborator

TkDodo commented Jun 4, 2024

there are conflicts now ...

@TkDodo
Copy link
Collaborator

TkDodo commented Jun 4, 2024

Please check for conflicts.

on your PR, please address the conflicts yourself. Thanks.

@gwansikk
Copy link
Contributor Author

gwansikk commented Jun 4, 2024

The conflicts were caused by file name changes in that PR, and now everything is perfect! @TkDodo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants