-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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(react-query): move away of uSES #8434
base: main
Are you sure you want to change the base?
Conversation
View your CI Pipeline Execution ↗ for commit 7e7d55c.
☁️ Nx Cloud last updated this comment at |
…/8384 # Conflicts: # packages/react-query/src/__tests__/useQuery.test.tsx
@@ -54,65 +54,6 @@ describe('useIsFetching', () => { | |||
await findByText('isFetching: 0') | |||
}) | |||
|
|||
it('should not update state while rendering', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing we should just trust React to optimize re-renders? 🤷
This reverts commit dcf16f2. # Conflicts: # packages/react-query/src/useBaseQuery.ts
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
// @ts-ignore | ||
global.IS_REACT_ACT_ENVIRONMENT = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
followed what Rick suggested here https://discord.com/channels/514829729862516747/1251229763838677052/1318328930658418718
{ | ||
"editor.defaultFormatter": "esbenp.prettier-vscode", | ||
"editor.formatOnSave": true | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to remove this, but it helps me as a contributor - I have a bunch of settings in trpc https://github.com/trpc/trpc/tree/next/.vscode
if (defaultedOptions.experimental_prefetchInRender) { | ||
// Do not notify on updates because of changes in the options because | ||
// these changes should already be reflected in the optimistic result. | ||
observer.setOptions(defaultedOptions, { listeners: false }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting the options straight away for experimental_prefetchInRender
seems to fix it.
I think the issue I had was some race condition...
I still have no reliable reproduction though
Unfortunately there's no way to avoid tearing without using uSES (except moving everything onto the react state). But this is fine, it's an acceptable drawback in some situations. Side note: maybe it worth introducing a separate hook for that? or a config option for those who doesn't want to have unavoidable tearing? But I'm pretty happy with this change, it's a huge improvement and removes some ugly hacks I'm using (like mocking the native impl of the uSES with the |
Closes #8384 (I think/hope !?!!?)
Refactor
@tanstack/react-query
to not rely onuseSyncExternalStore()
as it creates issues in concurrent renderingI think it works!? I added the repro in examples and I also tried to add a test to make it fail