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

Throwing an error from a component while server rendering changes the default value of a context if there is a provider in that tree. #14502

Closed
emmatown opened this issue Dec 29, 2018 · 7 comments

Comments

@emmatown
Copy link

Do you want to request a feature or report a bug?
Report a bug
What is the current behavior?
Throwing an error from a component while server rendering changes the default value of a context if there is a provider in that tree.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React. Paste the link to your JSFiddle (https://jsfiddle.net/Luktwrdm/) or CodeSandbox (https://codesandbox.io/s/new) example below:

https://codesandbox.io/s/03o0pnor6w

import React from "react";
import { renderToString } from "react-dom/server";

let Theme = React.createContext("light");

let Throw = () => {
  throw new Error("some error");
};

try {
  renderToString(
    <Theme.Provider value="dark">
      <Throw />
    </Theme.Provider>
  );
} catch (e) {}

// this should log "light" since it's the default value and
// there's no provider in the tree but it logs "dark" instead
console.log(renderToString(<Theme.Consumer>{x => x}</Theme.Consumer>));

What is the expected behavior?

Throwing an error from a component while server rendering shouldn't change the default value of context.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

I think this is broken in all versions of React with new context. (Here's a codesandbox of the issue with React 16.3.0, https://codesandbox.io/s/l945mq008l) It also only happens while server rendering, it works correctly in the browser (https://codesandbox.io/s/mm10ov8688)

@gaearon
Copy link
Collaborator

gaearon commented Jan 18, 2019

Do you mind creating a failing test for this please?

@emmatown
Copy link
Author

Done in #14621

@stevenbuccini
Copy link

Hey, I'd like to pick this up issue if it's not claimed yet.

@gaearon
Copy link
Collaborator

gaearon commented Jan 18, 2019

Sure.

@stevenbuccini
Copy link

Potentially relevant PRs: #12985, #14182.

In this case, when we add a new Provider, the provided value is being stored on a new Thread ID as described in #14182. This wouldn't be a problem, except it's clearly not being cleaned up correctly when the rendering is interrupted by an error.

This is my first time diving into the React internals, so I'm hoping people can point me a couple of things that I can't seem to figure out:

  • How/where the current context gets passed to the child function in Context.Consumer
  • Resources to understand server-side rendering lifecycle, specifically multithreading, partial rendering, and how errors short-circuit the rendering process.

Then I should be able to hop back in the debugger and knock this out. Thanks!

@gaearon
Copy link
Collaborator

gaearon commented Feb 8, 2019

Sorry, I don't expect to be able to help there, but if you figure it out let me know!

@emmatown
Copy link
Author

Looks like this was fixed by #14706.

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

3 participants