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

fix(types): infer the narrowest type with queryOptions #5805

Merged
merged 1 commit into from
Jul 31, 2023

Conversation

Andarist
Copy link
Contributor

fixes #5436

@vercel
Copy link

vercel bot commented Jul 29, 2023

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

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
query ⬜️ Ignored (Inspect) Jul 29, 2023 2:10pm

@nx-cloud
Copy link

nx-cloud bot commented Jul 29, 2023

☁️ Nx Cloud Report

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

📂 See all runs for this branch


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@codesandbox-ci
Copy link

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 f0bc02d:

Sandbox Source
@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

@codecov-commenter
Copy link

Codecov Report

Patch coverage has no change and project coverage change: -8.05% ⚠️

Comparison is base (e0aad73) 97.11% compared to head (f0bc02d) 89.07%.
Report is 634 commits behind head on beta.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##             beta    #5805      +/-   ##
==========================================
- Coverage   97.11%   89.07%   -8.05%     
==========================================
  Files          50       30      -20     
  Lines        2391      421    -1970     
  Branches      706       89     -617     
==========================================
- Hits         2322      375    -1947     
+ Misses         67       42      -25     
- Partials        2        4       +2     

see 80 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@TkDodo TkDodo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️🙏

@TkDodo TkDodo merged commit 1d36680 into TanStack:beta Jul 31, 2023
@TkDodo
Copy link
Collaborator

TkDodo commented Sep 11, 2023

@Andarist it seems that with this change, we lost excess property checks on the queryOptions function. For example, if I misspell a property (e.g. stallTime instead of staleTime), I'm not getting any errors:

TypeScript playground

We don't have a test for this, but I tried it out with the version from before that PR, and there, it used to work 🤔

@Andarist
Copy link
Contributor Author

This gets tricky - the inferred type is always exempt from the excess property check. We need to remember that the excess property check is more like a lint rule than anything else.

If you want to eat cake and have it too then perhaps this self-validation technique is your best bet: TS playground.

Other than that... a bizarre thing like this could work: TS playground. This infers each property separately~ to ensure that they were present in the input. Thanks to that we can keep the excess property check "as is" because the target object that is meant to report it is not the target of inference on its own. More work/thought would have to be put into this one to handle the optionality of input properties correctly.

@TkDodo
Copy link
Collaborator

TkDodo commented Sep 11, 2023

uuh, let me try out the validate approach, that looks great. I'll also add some tests for this. Thank you 🙏

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

Successfully merging this pull request may close these issues.

3 participants