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

Make Uses of ReactDebugCurrentFrame.getCurrentStack Reentrant #10188

Closed
sebmarkbage opened this issue Jul 15, 2017 · 2 comments
Closed

Make Uses of ReactDebugCurrentFrame.getCurrentStack Reentrant #10188

sebmarkbage opened this issue Jul 15, 2017 · 2 comments

Comments

@sebmarkbage
Copy link
Collaborator

I realized that the refactor to use ReactDebugCurrentFrame as a decoupled stack frame for error messages is not reentrant: https://github.com/facebook/react/pull/10105/files#r127573520

Fiber is not reentrant but other renderers might be. The synchronous server renderer is atm.

We should add tests for warnings between renderers, such as calling a server-render from within a client render, and update the set/reset callsites to use push/pop instead to account for that.

@gaearon
Copy link
Collaborator

gaearon commented Jul 11, 2018

I didn't end up adding push/pop to DebugFrame itself because relying on it would break ReactDOMServer 16.X + React 16.0, for example.

We can reconsider this later, but for now I'm keeping track of reentrancy in ReactDOMServer itself, and restore the outer stack implementation on exit from the outermost ReactDOMServer call.

I will later probably do something similar for cross-renderer calls on the client (except it's simpler because there's no re-entrancy within one renderer instance).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
@sebmarkbage @gaearon and others