Add regression tests for StrictMode transition unmount cleanup#36323
Open
Mouedrhiri wants to merge 1 commit intofacebook:mainfrom
Open
Add regression tests for StrictMode transition unmount cleanup#36323Mouedrhiri wants to merge 1 commit intofacebook:mainfrom
Mouedrhiri wants to merge 1 commit intofacebook:mainfrom
Conversation
|
Comparing: 94643c3...aef82ee Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: (No significant changes) |
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
2 participants
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.
Summary
This pull request adds regression coverage for issue #36284 about useEffect cleanup behavior when a component is unmounted during a transition under StrictMode with concurrent rendering.
Motivation:
The issue report describes an edge case where effect cleanup may be skipped, potentially causing leaked event listeners and duplicate subscriptions. Even though current behavior in the tested scenarios appears correct, this path did not have explicit focused regression protection for the exact StrictMode + transition unmount pattern.
What this PR changes:
Adds a StrictMode transition-unmount regression test at the reconciler level.
Adds a DOM-level regression test that mirrors the real-world pattern from the issue:
Effect registers a window event listener.
Component is unmounted using startTransition.
Test verifies listener is removed and cleanup is called.
No production runtime logic was changed in this PR.
This is a test-only hardening change to prevent future regressions.
Why this is valuable:
It codifies expected cleanup behavior in the exact scenario raised by the issue.
It protects both internal reconciler behavior and public DOM renderer behavior.
It improves confidence around StrictMode and concurrent transition unmount semantics.
How did you test this change?
I validated the change with focused targeted tests for the added regression coverage.
Commands run:
corepack yarn install --ignore-engines --force
corepack yarn test packages/react-reconciler/src/tests/ActivityStrictMode-test.js -t "calls passive cleanup when unmounting in a transition"
corepack yarn test packages/react-dom/src/tests/ReactDOMFiberAsync-test.js -t "calls effect cleanup when unmounting in a transition inside StrictMode"
Observed results:
Reconciler targeted test: PASS
React DOM targeted test: PASS
Both tests confirm cleanup executes and no post-unmount listener is left active in the tested StrictMode transition unmount scenarios.
Verification approach:
Assert cleanup is invoked on transition-driven unmount.
Assert a post-unmount event does not trigger the removed listener.
Account for StrictMode development double-invocation behavior in assertions.
Scope note:
I ran focused tests directly related to this fix. I did not run the full repository test suite, lint, Flow, or production test matrix in this local iteration.