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

Updating reactive outside of event handler causes state changes to propagate in wrong order #8014

Open
talha5389 opened this issue Apr 21, 2021 · 5 comments

Comments

@talha5389
Copy link

talha5389 commented Apr 21, 2021

Intended outcome:
Child should not see updated state change before parent similar to how React.useContext works

Actual outcome:
Child sees updated state before parent component and multiple renders are triggered even if only single reactive var is being changed.

How to reproduce the issue:
https://codesandbox.io/s/quirky-williamson-v5d0r?file=/src/App.js

  1. [CORRECT BEHAVIOR] Click on any of following buttons. You will notice in logs that new state is always visible to parent component and then in child component root -> (parent-reactive | parent-context) -> (child-reactive | child-context)
    1. Update React Context (this uses react context and works fine)
    2. Update React Context Async (this uses react context and works fine)
      • instead of updating context within handler this has setTimeout to simulate update outside of event handler
    3. Update Reactive (this updates reactive variable within context of event handler and works fine)
  2. [INCORRECT BEHAVIOR] Now click on Update Reactive Async
    • instead of updating context within handler this has setTimeout to simulate update outside of event handler
    • With this, child component see state change even before parent. State seem to transition like (child-reactive | child-context) -> (parent-reactive | parent-context) -> (child-reactive | child-context) -> root -> (parent-reactive | parent-context) -> (child-reactive | child-context)
    • I can confirm that if I wrap it with react dom unstable api for batching updates, it works as expected but I assume that it should have worked without it.
    • It leads to crashes where parent conditionally renders child based on state if child gets null in updated state before parent does. Here is sandbox to reflect just such scenario https://codesandbox.io/s/naughty-almeida-mow3i?file=/src/App.js

Versions

  • @apollo/client: 3.3.15
@jcreighton
Copy link
Contributor

Hi @talha5389! Thanks for the reproduction. It would be helpful if you could also provide the exact steps to reproduce the issue in that reproduction. I can make some assumptions but having steps to follow will help us narrow down what might happening here.

@talha5389
Copy link
Author

talha5389 commented Apr 22, 2021

@jcreighton Updated issue description and added another simplied sandbox to show real implication of it

@jcreighton
Copy link
Contributor

Thank you @talha5389! We'll take a look.

@brainkim brainkim self-assigned this Jun 22, 2021
@talha5389
Copy link
Author

talha5389 commented Jul 2, 2021

@jcreighton Any update?

@dejavoodooo
Copy link

Checking for updates here.

@hwillson hwillson added the 🔍 investigate Investigate further label May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants