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

[FIX] runtime: don't emit async hook warnings when cancelled/destroyed #1604

Merged
merged 1 commit into from
Apr 15, 2024

Conversation

sdegueldre
Copy link
Contributor

@sdegueldre sdegueldre commented Apr 12, 2024

Previously, the async hook warning that's emitted when async hooks take
too long to resolve would be emitted even if the component was
destroyed or cancelled. We used to check if the node's fiber was still
the current fiber as a proxy for cancellation, but this is insufficient
in the case where an app might be destroyed.

This commit also wraps the content of some of the warning tests into a
try/finally block so that a failure of these tests doesn't pollute
setTimeout and console.warn and cause other tests to timeout one after
another.

Previously, the async hook warning that's emitted when async hooks take
too long to resolve would be emitted even if the component was
destroyed or cancelled. We used to check if the node's fiber was still
the current fiber as a proxy for cancellation, but this is insufficient
in the case where an app might be destroyed.

This commit also wraps the content of some of the warning tests into a
try/finally block so that a failure of these tests doesn't pollute
setTimeout and console.warn and cause other tests to timeout one after
another.
@sdegueldre sdegueldre force-pushed the master-async-hooks-warning-fix-sad branch from 35be435 to dbd845f Compare April 15, 2024 07:57
@sdegueldre sdegueldre marked this pull request as ready for review April 15, 2024 07:58
@ged-odoo ged-odoo merged commit fddb1ec into master Apr 15, 2024
3 checks passed
@ged-odoo ged-odoo deleted the master-async-hooks-warning-fix-sad branch April 15, 2024 08:01
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.

2 participants