refactor(reactBatchedUpdates): remove the side effect that sets batchNotifyFn in react
This PR attempts to fix #4685 of v5 milestones
- set sideEffects to false in react-query/package.json
- remove the side effect import from index.ts
- delete reactBatchedUpdates.ts, reactBatchedUpdates.native.ts, setBatchUpdatesFn.ts files and references to them
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 e1dc8edfc58758ef07e4747ffe3302f84fb52cde:
| Sandbox | Source |
|---|---|
| @tanstack/query-example-react-basic-typescript | Configuration |
| @tanstack/query-example-solid-basic-typescript | Configuration |
| @tanstack/query-example-vue-basic | Configuration |
Thanks. I think we'll need to wait for the PR that removes the react17 test suite because in react17, the batching does make a difference :)
Thanks. I think we'll need to wait for the PR that removes the react17 test suite because in react17, the batching does make a difference :)
Awesome thanks! Yeah I was wondering if the solution should be backwards compatible, but looks like you decided to completely drop support for react 17
Codecov Report
:exclamation: No coverage uploaded for pull request base (
v5@22be6d5). Click here to learn what that means. Patch has no changes to coverable lines.
Additional details and impacted files
@@ Coverage Diff @@
## v5 #4699 +/- ##
=====================================
Coverage ? 92.27%
=====================================
Files ? 88
Lines ? 3585
Branches ? 941
=====================================
Hits ? 3308
Misses ? 262
Partials ? 15
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
react17 removal PR has been merged. can you solve the conflicts here please?
Also, please add some lines to the migration guide, especially for react-native, where this might be needed to set manually.
The merge conflict should be resolved. As for the migration guide, do we have any confirmation that this is still needed in react-native?
The merge conflict should be resolved. As for the migration guide, do we have any confirmation that this is still needed in react-native?
Not really, but it depends on which react-native "architecture" you're using. I think what we can write is something along the lines of:
If your Framework supports a custom batching function, you can let TanStack Query know about it by calling, for example:
import { unstable_batchedUpdates } from 'react-native'
import { notifyManager } from '@tanstack/react-query
notifyManager.setBatchNotifyFunction(unstable_batchedUpdates)
solid does this with a batch function now that is not unstable, we could also point to that:
https://github.com/TanStack/query/blob/b0d3c62070e8960ced79e6686c6c2f0154582a55/packages/solid-query/src/setBatchUpdatesFn.ts#L1-L4
Added the following to the v5 migration guide, please let me know if it needs any changes https://github.com/TanStack/query/pull/4699/commits/e1dc8edfc58758ef07e4747ffe3302f84fb52cde
perfect, thank you 🙏