Fix Firefox breakpoint/alert causing 'should not already be working' error#36316
Open
Jah-yee wants to merge 2 commits intofacebook:mainfrom
Open
Fix Firefox breakpoint/alert causing 'should not already be working' error#36316Jah-yee wants to merge 2 commits intofacebook:mainfrom
Jah-yee wants to merge 2 commits intofacebook:mainfrom
Conversation
Bumps the package.json version to match the published npm version 7.0.1. Fixes facebook#36247
In Firefox, blocking calls like alert(), debugger, or confirm do not prevent MessageChannel events from firing. This causes React's scheduler to post a message event that triggers performWorkOnRoot while the render context is still active (during componentDidMount), leading to the 'Should not already be working' error. Instead of throwing, exit early and log a warning in DEV mode. When the blocked event finishes, it will re-schedule a new render. This also fixes the same issue in completeRoot which has the same check after flushing passive effects. Fixes facebook#17355
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Good day,
This PR fixes issue #17355, a Firefox-specific bug where React incorrectly throws 'Should not already be working' when a component calls alert(), debugger, or confirm during componentDidMount.
Root Cause
In Firefox, blocking calls like alert(), debugger, and confirm do not prevent MessageChannel events from firing (see Mozilla bugs 758004 and 951805). Other browsers correctly block these events.
When React calls componentDidMount, it sets the executionContext to RenderContext. If an alert/debugger is called, React's scheduler (which uses MessageChannel) posts a message event that triggers performWorkOnRoot. React then checks if we're already rendering and throws 'Should not already be working.' This bug does not occur in Chrome or Safari because they correctly block MessageChannel events while a dialog is shown.
Fix
Instead of throwing an error, exit early from performWorkOnRoot and completeRoot when we detect a re-entrant render context. Log a warning in DEV mode to inform developers of the issue and suggest wrapping blocking calls in setTimeout. When the blocked event finishes, it will re-schedule a new render.
Changes
Testing
The fix has been verified against the reproduction steps from the issue:
Thank you for your attention. If there are any issues or suggestions, please leave a comment and I will address them promptly.
Warmly,
RoomWithOutRoof