Does anyone have reliable reproduction steps? We can't really help without them.

I has this issue and the problem was related to an ajax call with jquery made in componentDidMount with async parameter to false, the réponse of tha call updated the state, this causes some issues with latest react release to synchronise the state I assume. setting async to true solved the issue

See codesandbox above - run in firefox.

Oh nice. Thanks.

All right, so the consistent repro is:

1. Open https://codesandbox.io/s/should-not-already-be-working-bl72j?file=/src/index.js in Firefox
2. Observe the error message

There is some kind of timing assumption that gets broken by the alert() being a blocking call. I don't know where the problem is. Does anyone want to dive in the source and figure it out?

@gaearon I can have a look at this

Sure, thanks!

Hi, guys!

I ran into this problem on my app and thankfully, this issue is already raised here.

I tried to debug the issue locally but I'm unable to reproduce the issue in my local build. Here's what I did:

1. I included the code sample mentioned here by @mooflu in my App.js file in fixtures/dom/src/components.
2. I started the DOM fixtures server according to the README.
3. However, I cannot reproduce the error locally even when selecting the production build. But, the error is triggered when I choose any remote React version. I've attached a snippet below to show the behavior.

Based on the error trace, it seems like the error was triggered at performSyncWorkOnRoot:

I've done a quick investigation and it seems like the executionContext bit value is not equal to NoContext. I assume this means executionContext is still in the rendering stage? Here are the pics:

Meanwhile, here are the context bits in Chrome at the same breakpoint:

I'd be happy to investigate further into this if no one is taking it up, although it may take some time as I'm unfamiliar with React internals. Also, is there a way for me to test the production version locally e.g. using the production build to test the dev.html file in fixtures/packaging/babel-standalone?

Thanks!

@julianfssen i have been busy lately I think you have made some progress, I guess you can take it up from here

@julianfssen i have been busy lately I think you have made some progress, I guess you can take it up from here

Hey @mdaj06 ! Sorry for the slow reply. I apologize as I didn't see your comment about taking the issue up, in that case I will help you take a look at this and see what I can do. Thanks!

My site is working in Chrome fine.. not even a warning.
but in Firefox, it says Error: Should not already be working. The error is on my app.js file where i used class component and used component did mount.

but after copying my cookies from Chrome to Firefox.. the site started working.
so I think this has some relationships to cookies.. or local site data.

My site is working in Chrome fine.. not even a warning.
but in Firefox, it says Error: Should not already be working. The error is on my app.js file where i used class component and used component did mount.

Yup! This seems to only trigger in Firefox, I've investigated the issue and I think I may have found the cause, will post a writeup here on it in awhile to be verified by the team. In the meantime, you can wrap the alert() call in a setTimeout() block e.g:

setTimeout(() => console.log('your message'), 0)

to 'fix' it.

TL;DR: Firefox does not block WebSockets execution for alert and debugger calls, causing the scheduler to performWorkOnDeadline incorrectly and setting the wrong executionContext when calling performSyncWorkOnRoot. Microtasks solve this problem, but flushing via flushSyncCallbackQueue still causes the same issue.

I had a look at this issue and made a few observations about why this is happening.

This issue happens on Firefox for React v16.10 to the latest v17.02 release. In v16.10, @acdlite added the feature to use message channels to manage the cycle between performing work and yielding to the browser.

This works perfectly on every browser except Firefox, which has an issue with it. window.alert() is a blocking call and works correctly most of the times. However, it does not block the execution of WebSocket calls, which is not the case with Chrome and other browsers. Here are some bug reports I found regarding this issue:

• WebSocket callbacks can trigger while alert() is displayed, breaking execution order of my code and causing corruption.
• Alert does not block UI thread execution.

One way to observe the bug is to run the fixtures/dom mini-app and modify your App.js like so (thanks to @mooflu for the code I used to replicate this behavior:

Try to change the React version and observe the behavior. If you pulled the latest commit as of today, there should be no issues for your local environment. There should also be no issues for v16.10 and below.

Below are snippets to demonstrate the behavior.

Observe the behavior and console logs in each browser.

Behavior on Chrome:

Behavior on Safari:

Behavior on Firefox (buggy):

After calling portMessage in Firefox, the execution does not seem to pause as expected when alert(), unlike other browsers. performWorkUntilDeadline is called and this messes up the executionContext when calling performSyncWorkOnRoot.

However, I was curious why this happened in production but not when cloning the repo and testing it locally. It turns out this issue is fixed with the enableSyncMicroTasks feature to flush tasks instead of flushSyncCallbackQueue which seems to be one invoking the message issue above. I tested the branch above and the issue does not happen in Firefox with alert() and debugger.

There should be no issues when using microtasks, but the should not already be working error should trigger if you remove the supportsMicrotasks flag. Remove the supportsMicrotasks check to fallback to the previous non-microtask way of flushing the queue—and the issue will appear again, possibly due to flushSyncCallbackQueue.

Try checking out the branch (or the latest commit) and replicate the steps above.

How to replicate the issue:

1. There are two ways to test this. You can run the mini-app in fixtures/dom or use the babel-standalone/dev.html.Add the following snippet to your App.js file if you're using the former, or add it to your <script> tag for the latter.

class AppClass extends React.Component { // don't forget to export the class if you're calling <AppClass />
  constructor(props) {
    super(props);
    this.state = {
      num: 99
    };
  }

  componentDidMount() {
    this.setState((state) => {
      return { num: state.num + 2 };
    });
    alert('AppClass');
  }

  render() {
    return (
      <div className="App">
        <h1>Num: {this.state.num}</h1>
      </div>
    );
  }
}

2. Run the fixtures/dom server or open babel-standalone/dev.html in Firefox.
3. Observe the console. There should be an should not already be working error. Note: This error does not appear on v16.9.0 and below or if your branch is later than the land microTasks commit (although the issue still exists).
4. Disable the supportMicrotasks check in ensureRootIsScheduled (line 688 in both ReactFiberWorkLoop.old.js and ReactFiberWorkLoop.new.js).
5. Rebuild React with yarn build and try restarting the server or refreshing the fixture file again. The error should appear now in Firefox.

However, I'm not sure whether this is the real issue, despite my discoveries. @acdlite, may I know if these assumptions are aligned with your intentions when making the changes and if the replication steps is correct? Your commits helped a lot in drilling down the issue but I'm not familiar with React internals, so don't want to explain or assume the wrong things.

@julianfssen, massive thanks for getting to the bottom of this.

We're a bit overloaded with other things right now so I don't know if we'll take a detailed look very soon. You already gathered a lot of context for this issue — do you have a proposed fix in mind?

@julianfssen Thanks for the detailed write up!

So based on your description, Firefox has a bug where alert and debugger will pause the current event but will still spin the event loop to fire the next pending event, including message events which React uses to flush scheduled work. Then React throws an error because it detects that the previous render (the one with the debugger) hasn't finished yet.

My guess for why supportsMicrotask "fixes" the issue is that microtasks are part of the current event, so the debugger actually does block those from firing.

Perhaps there is a way we could detect this scenario in development and fail more gracefully. I'm loath to add more code to the production build to address something that is ultimately a Firefox bug, and one that only happens during development (unless you use an alert in prod, I guess, but that's not that common).

I would be curious to know if the prompt triggered by beforeunload causes the same issue because we don't use alert in production but we see some occurrence of this error. Can also be confirm which is probably similar.

Yea that's the one that got me worried. Confirm is pretty common for "do you want to stay on this page?" flows.

Right, prompt seems legit.

So the fix for this would be pretty simple, I think — instead of throwing an error, we'd exit from the function. I think this would work because when the paused event continues and eventually finishes, it will re-schedule a new render to replace the one that we exited from.

But this is a bit fragile, and I'd rather not commit to supporting this workaround long term. Hopefully we can convince the Firefox team to fix the underlying issue.

@acdlite thanks for explanation! That makes sense, and I agree that altering logic to cater for a specific browser bug does not seem 'right'. However, the event loop bug in Firefox has been there for over a decade now so I'm not sure if it's going to be fixed anytime soon. Based on my tests, the microtasks implementation works correctly for the behavior above. Since most modern browsers support microtasks, perhaps this is 'fixed' if the implementation goes in the next release?

@alexandcote thanks for pointing that out, I forgot about confirm and prompt when testing. I believe the issue happens in Firefox if the modal is meant to pause execution, so prompt and confirm will likely trigger the same issue since it (should?) works similarly to alert. I can test it this weekend.

@gaearon thanks! I'd love to suggest a fix but my knowledge of React internals is not good at the moment. I'll have a look again and tinker around with the implementation this weekend based on @acdlite 's feedback. That said, if anyone wants to take over this issue, go ahead! The issue relates to WebSockets not pausing on calls that are supposed to block execution (e.g. alert, debugger, confirm, etc.) but there may be more to discover about it.

We just ran across this in our Bugsnag logs this morning but it was ONLY for Chrome 83.0.4103 just like @mooflu. Maybe there is a bug in Chrome 83.0.4103 as well?

It might be related to this code -

  const [ready, toggleReady] = useState(false);
  useEffect(() => {
    if (branchKey && window.branch) {
      try {
        window.branch.init(branchKey, branchError => {
          if (!branchError) {
            toggleReady(true);
          }
        });
      } catch (branchError) {
        logError(branchError);
      }
    }
  }, [branchKey]);

Same error "Should not already be working" in Firefox but not Brave/Chrome (on Linux).
Problem code was trying to open a window (for polling login) in componentDidMount.

Problem goes away when I wrap the call in setTimeout.

setTimeout(function() {
				that.createLoginIframe()
			}, 1000)

I am getting the same behaviour in Firefox, when React is rendering components from within an IntersectionObserver and rendering components from outside of the same IntersectionObserver at the same time. Using window.setTimeout(renderCall, 0) from within the IntersectionObserver solves the issue.

I also noticed this is only happening, if the component NOT rendered through IntersectionObserver is making a synchronous network call! I managed to duplicate the issue here:

https://codesandbox.io/s/should-not-already-be-working-x1syi?file=/src/App.js

I am guessing this observed behaviour in Firefox is related to the discussions in here.