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

Feature/react 18 #3064

Merged
merged 191 commits into from
Apr 2, 2022
Merged

Feature/react 18 #3064

merged 191 commits into from
Apr 2, 2022

Conversation

TkDodo
Copy link
Collaborator

@TkDodo TkDodo commented Dec 7, 2021

No description provided.

TkDodo added 30 commits October 28, 2021 21:41
update react dependencies and add use-sync-external-store polyfill
use a version of uSES that actually has an implementation other than "Not Yet Implemented"
looks like we also need the experimental version of react, because v18-alpha doesn't support uSES yet.
update testing-library to v13 alpha
do not update currentResult when it is equal to the previousResult, because we use the currentResult as snapshot for uSES, so it must be as stable as possible
switch forceUpdate with uSES.

- I'm not sure if the `updateResult` effect is still necessary, or if it's guaranteed that we can't miss any updates because we don't subscribe in an effect anymore; tests behave the same with / without it - tbd
- subscribe must be stable, or else we wind up in an infinite loop. in order to be able to pass `observer.subscribe`, we must bind the function in the constructor
make the first test a bit more stable - we don't want more than 2 results
fix type issues in devtools tests, so we adhere to the new typings of testing-library v13
make devtools test more resilient: act throws an error in the latest version if used liked that, but we don't need it. We can just click the button and use waitFor, as documented here: https://testing-library.com/docs/guide-disappearance#2-using-waitfor
don't re-assign result
bring back the optimistic result; this is debatable because it means we actually _ignore_ whe result returned by uSES, but it makes for fewer re-renders as we can go back to silently update from the effect
useIsFetching to uSES

I don't fully understand the test that needed adaption, but the new numbers actually look more correct. The first thing that happens is showing the SecondQuery (after 50ms), and at that time, the FirstQuery is already fetching, so why should there be two zeros in the result array ...

judging from the console mock assertion, we are testing if state hasn't been updated on an unmounted component, which now can't happen anymore with uSES, so we can remove it
useIsMutatating to uSES

As a positive side-effect, there seem to be fewer re-renders now - the new numbers in the tests do make sense
useMutation to uSES

one big change is moving `setOptions` into a useEffect - similar to what `useQuery` is doing. However, we have no `getOptimisticResult` in useMutation, so we'll have to see how this behaves

the tests need some love - it's generally working, but the way the tests are written, we're getting some failure.
wait for heading to to to value `3` before asserting the onSuccess / onSettled calls
rewrite test to getByRole
since we're not returning anything from onError or onSettled in the tests, the mutation updates the data on the screen before the callbacks have finished running, which is why the test needs to waitFor the callbacks to have been called
work around console error from uSES by moving the console mock to the client part and / or increasing the assertion count for now
there seems to be one less rendering, likely because of batching, getting rid of one render that has the same assertions as the previous state, which is nice
update to v18 alpha, which should had the native uSES impl
count renders correctly by incrementing the count in useEffect
bump everything and import from /shim
make test more resilient by not using fireEvent
use findByText for more resilient tests
test against react 17 and react 18
only run bundlewatch once
give a better name
TkDodo added 2 commits March 25, 2022 13:55
it's optional and we don't do anything meaningful as of now in it; will need to re-add it once we do react18 hydration
because the latest alpha dropped support for react17
@najamelan
Copy link

najamelan commented Mar 29, 2022

React 18 has been released today and is not compatible with react-query according to npm:

npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR! 
npm ERR! While resolving: [email protected]
npm ERR! Found: [email protected]
npm ERR! node_modules/react
npm ERR!   react@"^18" from the root project
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peer react@"^16.8.0 || ^17.0.0" from [email protected]
npm ERR! node_modules/react-query
npm ERR!   react-query@"^3.34.19" from the root project

@wtchnm wtchnm mentioned this pull request Mar 29, 2022
5 tasks
@TkDodo TkDodo marked this pull request as ready for review March 30, 2022 07:15
@TkDodo TkDodo changed the base branch from alpha to beta April 2, 2022 11:41
@TkDodo TkDodo merged commit 4d753c0 into TanStack:beta Apr 2, 2022
@TkDodo TkDodo deleted the feature/react-18 branch April 2, 2022 11:42
@tannerlinsley
Copy link
Collaborator

🎉 This PR is included in version 4.0.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

7 participants