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(query-core): improve type safety and performance in getMutationDefaults #8669

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

samuel871211
Copy link

@samuel871211 samuel871211 commented Feb 19, 2025

Description

Improve type safety and performance in getMutationDefaults by:

  1. Using explicit return type OmitKeyof<MutationObserverOptions<any, any, any, any>, 'mutationKey'> to ensure the return type matches the type defined in setMutationDefaults
  2. Aligning implementation with getQueryDefaults by using Object.assign

…tionDefaults`

- Change return type to exclude mutationKey using OmitKeyof
- Replace spread operator with Object.assign for better performance
Copy link

nx-cloud bot commented Feb 28, 2025

View your CI Pipeline Execution ↗ for commit af7aa1e.

Command Status Duration Result
nx affected --targets=test:sherif,test:knip,tes... ✅ Succeeded 4m 8s View ↗
nx run-many --target=build --exclude=examples/*... ✅ Succeeded 1m 59s View ↗

☁️ Nx Cloud last updated this comment at 2025-02-28 19:33:50 UTC

Comment on lines -595 to +598
result = { ...result, ...queryDefault.defaultOptions }
Object.assign(result, queryDefault.defaultOptions)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Aligning implementation with getQueryDefaults by using Object.assign

@Nick-Lucas do you remember why you changed this to Object.assign in getQueryDefaults ?

Copy link
Contributor

@Nick-Lucas Nick-Lucas Feb 28, 2025

Choose a reason for hiding this comment

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

Yes, it's potentially a faster operation and in some cases looks cleaner. That's it. But the difference in speed is probably virtually irrelevant as both approaches are very fast

https://www.measurethat.net/Benchmarks/Show/2485/0/spread-vs-objectassign#:~:text=Executions%20per%20second-,Object.assign%20two%20objects,226555.7%20Ops/sec,-Object.assign%20three (note: the "AI summary" on this page doesn't understand the results and is confidently hallucinating the opposite results of what the data shows which is funny but confusing)

Copy link

pkg-pr-new bot commented Feb 28, 2025

Open in Stackblitz

More templates

@tanstack/angular-query-devtools-experimental

npm i https://pkg.pr.new/@tanstack/angular-query-devtools-experimental@8669

@tanstack/angular-query-experimental

npm i https://pkg.pr.new/@tanstack/angular-query-experimental@8669

@tanstack/eslint-plugin-query

npm i https://pkg.pr.new/@tanstack/eslint-plugin-query@8669

@tanstack/query-async-storage-persister

npm i https://pkg.pr.new/@tanstack/query-async-storage-persister@8669

@tanstack/query-broadcast-client-experimental

npm i https://pkg.pr.new/@tanstack/query-broadcast-client-experimental@8669

@tanstack/query-core

npm i https://pkg.pr.new/@tanstack/query-core@8669

@tanstack/query-devtools

npm i https://pkg.pr.new/@tanstack/query-devtools@8669

@tanstack/query-persist-client-core

npm i https://pkg.pr.new/@tanstack/query-persist-client-core@8669

@tanstack/query-sync-storage-persister

npm i https://pkg.pr.new/@tanstack/query-sync-storage-persister@8669

@tanstack/react-query

npm i https://pkg.pr.new/@tanstack/react-query@8669

@tanstack/react-query-devtools

npm i https://pkg.pr.new/@tanstack/react-query-devtools@8669

@tanstack/react-query-next-experimental

npm i https://pkg.pr.new/@tanstack/react-query-next-experimental@8669

@tanstack/react-query-persist-client

npm i https://pkg.pr.new/@tanstack/react-query-persist-client@8669

@tanstack/solid-query

npm i https://pkg.pr.new/@tanstack/solid-query@8669

@tanstack/solid-query-devtools

npm i https://pkg.pr.new/@tanstack/solid-query-devtools@8669

@tanstack/solid-query-persist-client

npm i https://pkg.pr.new/@tanstack/solid-query-persist-client@8669

@tanstack/svelte-query

npm i https://pkg.pr.new/@tanstack/svelte-query@8669

@tanstack/svelte-query-devtools

npm i https://pkg.pr.new/@tanstack/svelte-query-devtools@8669

@tanstack/svelte-query-persist-client

npm i https://pkg.pr.new/@tanstack/svelte-query-persist-client@8669

@tanstack/vue-query

npm i https://pkg.pr.new/@tanstack/vue-query@8669

@tanstack/vue-query-devtools

npm i https://pkg.pr.new/@tanstack/vue-query-devtools@8669

commit: af7aa1e

Copy link

codecov bot commented Feb 28, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.11%. Comparing base (2bf7ec6) to head (af7aa1e).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #8669       +/-   ##
===========================================
+ Coverage   46.31%   63.11%   +16.79%     
===========================================
  Files         199      135       -64     
  Lines        7552     4855     -2697     
  Branches     1731     1368      -363     
===========================================
- Hits         3498     3064      -434     
+ Misses       3674     1546     -2128     
+ Partials      380      245      -135     
Components Coverage Δ
@tanstack/angular-query-devtools-experimental ∅ <ø> (∅)
@tanstack/angular-query-experimental 88.65% <ø> (ø)
@tanstack/eslint-plugin-query ∅ <ø> (∅)
@tanstack/query-async-storage-persister 43.85% <ø> (ø)
@tanstack/query-broadcast-client-experimental ∅ <ø> (∅)
@tanstack/query-codemods ∅ <ø> (∅)
@tanstack/query-core 94.07% <100.00%> (ø)
@tanstack/query-devtools 4.78% <ø> (ø)
@tanstack/query-persist-client-core 57.73% <ø> (ø)
@tanstack/query-sync-storage-persister 84.61% <ø> (ø)
@tanstack/react-query 95.09% <ø> (ø)
@tanstack/react-query-devtools 10.00% <ø> (ø)
@tanstack/react-query-next-experimental ∅ <ø> (∅)
@tanstack/react-query-persist-client 100.00% <ø> (ø)
@tanstack/solid-query 78.20% <ø> (ø)
@tanstack/solid-query-devtools ∅ <ø> (∅)
@tanstack/solid-query-persist-client 100.00% <ø> (ø)
@tanstack/svelte-query 87.33% <ø> (ø)
@tanstack/svelte-query-devtools ∅ <ø> (∅)
@tanstack/svelte-query-persist-client 100.00% <ø> (ø)
@tanstack/vue-query 71.01% <ø> (ø)
@tanstack/vue-query-devtools ∅ <ø> (∅)

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.

3 participants