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

Try to update refs immediately rather than useIsomorphicLayoutEffect #1438

Closed
wants to merge 1 commit into from
Closed

Try to update refs immediately rather than useIsomorphicLayoutEffect #1438

wants to merge 1 commit into from

Conversation

malash
Copy link

@malash malash commented Oct 29, 2019

This PR tries to fix #1437

I'v tested it locally and it works well.

But there is still something todo.

  1. How to add unit tests for this PR ? useIsomorphicLayoutEffect cannot pass test cases in non-web environment #1436
  2. Would this break anything current test cases haven't covered ?

@netlify
Copy link

netlify bot commented Oct 29, 2019

Deploy preview for react-redux-docs ready!

Built with commit 81581b0

https://deploy-preview-1438--react-redux-docs.netlify.com

@markerikson
Copy link
Contributor

markerikson commented Oct 29, 2019

Appreciate the PR, but no, this logic is incorrect. We cannot directly mutate refs while rendering, as that will cause (more) problems in Concurrent Mode.

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

Successfully merging this pull request may close these issues.

Dispatch more than once in non-batched update cause incorrect result with useSelector
2 participants