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

refactor[react-devtools/extensions]: dont debounce cleanup logic on navigation #30027

Merged

Conversation

hoxyq
Copy link
Contributor

@hoxyq hoxyq commented Jun 21, 2024

Summary

There is a race condition in the way we poll if React is on the page and when we actually clear this polling instance. When user navigates to a different page, we will debounce a callback for 500ms, which will:

  1. Cleanup previous React polling instance
  2. Start a new React polling instance

Since the cleanup logic is debounced, there is a small chance that by the time we are going to clean up this polling instance, it will be eval-ed on the page, that is using React. For example, when user is navigating from the page which doesn't have React running, to a page that has React running.

Next, we incorrectly will try to mount React DevTools panels twice, which will result into conflicts in the Store, and the error will be shown to the user

How did you test this change?

Since this is a race condition, it is hard to reproduce consistently, but you can try this flow:

  1. Open a page that is using React, open browser DevTools and React DevTools components panel
  2. Open a page that is NOT using React, like google.com, wait ~5 seconds until you see "Looks like this page doesn't have React, or it hasn't been loaded yet" message in RDT panel
  3. Open a page that is using React, observe the error "Uncaught Error: Cannot add node "1" because a node with that id is already in the Store."

Couldn't been able to reproduce this with these changes.

@hoxyq hoxyq requested a review from vzaidman June 21, 2024 15:40
Copy link

vercel bot commented Jun 21, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 21, 2024 3:54pm

@hoxyq hoxyq force-pushed the react-devtools/dont-debounce-cleanup-logic branch from a35b3bb to 1744c01 Compare June 21, 2024 15:52
@hoxyq hoxyq merged commit 7608516 into facebook:main Jun 24, 2024
44 checks passed
@hoxyq hoxyq deleted the react-devtools/dont-debounce-cleanup-logic branch June 24, 2024 12:28
hoxyq added a commit that referenced this pull request Jul 3, 2024
## Summary

Full list of changes, mostly fixes:
* chore[react-devtools/renderer]: dont show strict mode warning for prod
renderer builds ([hoxyq](https://github.com/hoxyq) in
[#30158](#30158))
* chore[react-devtools/ui]: fix strict mode badge styles
([hoxyq](https://github.com/hoxyq) in
[#30159](#30159))
* fix[react-devtools]: restore original args when recording errors
([hoxyq](https://github.com/hoxyq) in
[#30091](#30091))
* Read constructor name more carefully
([LoganDark](https://github.com/LoganDark) in
[#29954](#29954))
* refactor[react-devtools/extensions]: dont debounce cleanup logic on
navigation ([hoxyq](https://github.com/hoxyq) in
[#30027](#30027))
* lint: enable reportUnusedDisableDirectives and remove unused
suppressions ([kassens](https://github.com/kassens) in
[#28721](#28721))
* fix[react-devtools/extensions]: propagate globals from env
([hoxyq](https://github.com/hoxyq) in
[#29963](#29963))
* refactor[react-devtools/tests]: use registered marks instead of
cleared in tests ([hoxyq](https://github.com/hoxyq) in
[#29929](#29929))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants