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

Solution to "Should not already be working" Issue in Firefox after Breakpoint/Alert #31945

Conversation

amirmousazadeh-1984
Copy link

Problem Description:
The Issue: When using alert or debugger in Firefox, scheduled operations (such as setTimeout or requestIdleCallback) do not execute properly after breakpoints, leading to the error "Should not already be working."

Explanation: This issue typically arises when setTimeout or similar timing methods are used within React code. When a breakpoint or alert is triggered during the execution of these methods, the scheduled tasks can be interrupted, causing them to fail to run after the breakpoint has been cleared.

Impact of the Problem:
Performance Disruptions: The failure to execute scheduled operations after a breakpoint or alert disrupts the overall functioning of the application, causing performance issues.

Browser-Specific Issue: This issue primarily affects Firefox and is noticeable to developers who are debugging their applications, which leads to a negative development and debugging experience.

Proposed Solutions:
To address this issue, the following solutions are suggested:

Using queueMicrotask for task scheduling: The queueMicrotask method is a more reliable way to schedule microtasks that execute immediately after the current task completes, before any other events like alert or debugger. This can help avoid issues after breakpoints.

Fallback to requestIdleCallback: If the browser supports requestIdleCallback, it can be used to schedule tasks during idle periods, ensuring that tasks execute when the browser is less busy, thus preventing interruptions.

Using Date.now() for more precise scheduling: In some cases, using Date.now() instead of setTimeout can provide better control over the timing of operations, avoiding potential issues caused by interruption.

Managing task queue with a custom task queue: Implementing a custom queue for tasks could ensure operations are executed in order and without interruption, particularly when alert or debugger is used during the execution of scheduled tasks.

Benefits of These Changes:
Stability after Breakpoints: These changes would ensure that operations continue executing correctly after a breakpoint or alert in Firefox, addressing the "Should not already be working" issue.

Non-Interfering Execution: Tasks would be executed in a non-interfering manner, ensuring that scheduled tasks complete as intended without being disrupted.

Improved Developer Experience: Developers would no longer face the issue of tasks failing to run after breakpoints, resulting in a smoother debugging and development process.

Proposed Code Example:
The following code snippet demonstrates how the changes could be applied:
function requestHostCallback() {
if (!isMessageLoopRunning) {
isMessageLoopRunning = true;
schedulePerformWorkUntilDeadline();
}
}

function requestHostTimeout(
callback: (currentTime: number) => void,
ms: number,
) {
// Use queueMicrotask instead of setTimeout
queueMicrotask(() => {
callback(getCurrentTime());
});
}

function cancelHostTimeout() {
// No significant change needed here
localClearTimeout(taskTimeoutID);
taskTimeoutID = ((-1: any): TimeoutID);
}

Conclusion:
By implementing these changes, we can resolve the "Should not already be working" issue in Firefox after breakpoints and ensure that scheduled tasks continue executing without issues. These adjustments would improve the stability and reliability of React’s asynchronous scheduling system, providing a better experience for both developers and end users.

Should not already be working" in Firefox after a breakpoint/alert
Copy link

vercel bot commented Dec 31, 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 Dec 31, 2024 0:05am

@facebook-github-bot
Copy link

Hi @amirmousazadeh-1984!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

Copy link

@DuoAlly-AI-Recruiter DuoAlly-AI-Recruiter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice coding.

callback(getCurrentTime());
}, ms);
});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This callsite is trying to schedule a delayed task in the future, not a microtask. So this would break a lot of things.

@sebmarkbage
Copy link
Collaborator

React should never be calling this codepath by itself. It really only exists for Meta's polyfilling of timers for better scheduling performance. The plan is to remove the delay option all together since we'd now favor native postTask anyway.

So I'm not sure how you'd ever hit this code path. Probably needs a bug report with a repro.

@sebmarkbage sebmarkbage closed this Jan 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants