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

Bug: Context.Consumer inside Suspense does not receive context updates while suspended #19701

Closed
overlookmotel opened this issue Aug 26, 2020 · 5 comments · Fixed by #23095
Closed

Comments

@overlookmotel
Copy link
Contributor

A <Context.Consumer> callback inside a suspended <Suspense> boundary does not receive updates when context value changes.

This issue was originally found with React Router (remix-run/react-router#7137) but can be reproduced with React alone.

React version: 16.13.1 or 17.0.0-rc.0 (and at least some earlier versions too)

Steps To Reproduce

https://codesandbox.io/s/suspense-context-test-case-6jlsq

The current behavior

In the example above, clicking on the "About" button puts the <Suspense> boundary into a suspended state. Clicking "Home" updates the context value, but the <Context.Consumer> callback is not called with this updated value. Therefore the page never navigates back to "Home".

Please note that Received context: home is not logged after the "Home" button is pressed.

The expected behavior

The <Context.Consumer> callback should be called with value 'home' when the value of the context is updated.

Please note that the problem does not occur if any of:

  1. the Suspense boundary is never suspended (uncomment the first line of About function to verify).
  2. useContext() is used in place of <Context.Consumer> (substitute <SwitchWithUseContext /> for <Switch />).
  3. the contents of Switch function is inlined within the <Suspense> rather than in a separate component.

The last of these leads me to suspect that the problem may be related to #17356. In the example, <Context.Consumer> is within a Switch component. But the <Switch> element inside <Suspense> never changes - so it's effectively memoized. I wonder if this is why it doesn't receive context updates?

@overlookmotel overlookmotel added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Aug 26, 2020
@gaearon
Copy link
Collaborator

gaearon commented Aug 26, 2020

Can you turn this into a failing test, like we did for #17356?

@overlookmotel
Copy link
Contributor Author

overlookmotel commented Aug 26, 2020

@gaearon Yup! #19702

overlookmotel added a commit to overlookmotel/react that referenced this issue Aug 26, 2020
@engprodigy
Copy link

engprodigy commented Sep 25, 2020

@overlookmotel and @gaearon This is looking abandoned. Can I take a look at it?

@overlookmotel
Copy link
Contributor Author

@engprodigy I've submitted PR #19702 with a failing test. Personally, I'd very much appreciate your input if you have any ideas for a fix.

@engprodigy
Copy link

Sure @overlookmotel

gaearon pushed a commit to gaearon/react that referenced this issue Jan 11, 2022
gaearon added a commit that referenced this issue Jan 19, 2022
* Failing test for Context.Consumer in suspended Suspense

See issue #19701.

* Fix context propagation for offscreen trees

* Address nits

* Specify propagation root for Suspense too

* Pass correct propagation root

* Harden test coverage

This test will fail if we remove propagation, or if we propagate with a root node like fiber.return or fiber.return.return. The additional DEV-only error helps detect a different kind of mistake, like if the thing being passed hasn't actually been encountered on the way up. However, we still leave the actual production loop to check against null so that there is no way we loop forever if the propagation root is wrong.

* Remove superfluous warning

Co-authored-by: overlookmotel <[email protected]>
zhengjitf pushed a commit to zhengjitf/react that referenced this issue Apr 15, 2022
* Failing test for Context.Consumer in suspended Suspense

See issue facebook#19701.

* Fix context propagation for offscreen trees

* Address nits

* Specify propagation root for Suspense too

* Pass correct propagation root

* Harden test coverage

This test will fail if we remove propagation, or if we propagate with a root node like fiber.return or fiber.return.return. The additional DEV-only error helps detect a different kind of mistake, like if the thing being passed hasn't actually been encountered on the way up. However, we still leave the actual production loop to check against null so that there is no way we loop forever if the propagation root is wrong.

* Remove superfluous warning

Co-authored-by: overlookmotel <[email protected]>
nevilm-lt pushed a commit to nevilm-lt/react that referenced this issue Apr 22, 2022
* Failing test for Context.Consumer in suspended Suspense

See issue facebook#19701.

* Fix context propagation for offscreen trees

* Address nits

* Specify propagation root for Suspense too

* Pass correct propagation root

* Harden test coverage

This test will fail if we remove propagation, or if we propagate with a root node like fiber.return or fiber.return.return. The additional DEV-only error helps detect a different kind of mistake, like if the thing being passed hasn't actually been encountered on the way up. However, we still leave the actual production loop to check against null so that there is no way we loop forever if the propagation root is wrong.

* Remove superfluous warning

Co-authored-by: overlookmotel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants