[internal] add feature flag to fix event handle API in Activity - #35562
[internal] add feature flag to fix event handle API in Activity#35562rickhanlonii wants to merge 1 commit into
Conversation
|
Comparing: 3aaab92...c2cc16c 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) |
|
@Ranamalsingh12 future AI generated comment spam will result in a ban. |
| shouldFireAfterActiveInstanceBlur = true; | ||
| beforeActiveInstanceBlur(finishedWork); | ||
| } | ||
| } |
There was a problem hiding this comment.
Actually, this might double blur since Suspense uses Activity - let me look into it more
b74caa8 to
c2cc16c
Compare
| }); | ||
|
|
||
| if (withMemo) { | ||
| // BUG: when suspended children are not updated blur is not fired. |
There was a problem hiding this comment.
This test shows an existing bug in Suspense where we don't fire blur if the children are not updated while suspending (the memo() case).
| 'beforeblur - parent', | ||
| 'beforeblur - child', | ||
| 'afterblur - parent', | ||
| ]); |
There was a problem hiding this comment.
I actually don't know why this works in the memo() case. It should be the same as Supense - the children shouldn't be visited because of the memo so no event fired?
|
|
||
| expect(log).toEqual([ | ||
| // BUG: we already fired blur events for the hidden element. | ||
| // This is a bug that's always existed. |
There was a problem hiding this comment.
Another existing Suspense bug - blur fired twice if the tree suspends while already suspended.
|
|
||
| if (withMemo && gate('enableEventAPIActivityFix')) { | ||
| // Bug in new flag: if the `withMemo` case above is fixed (so blur always fires), | ||
| // this would be wrong. But as long as both bugs exist, this seems right. |
There was a problem hiding this comment.
This shit is crazy broken
| if (gate('enableEventAPIActivityFix')) { | ||
| expect(log).toEqual([ | ||
| // BUG: unlike the last test, these cause double events if the children are not memoized. | ||
| // Seems pretty bad, idk. |
There was a problem hiding this comment.
This bug is a blocker for this flag. I'm amazed this thing is even in prod.
Noticed this TODO was never implemented, so this implements support for Activity inside the deprecated, never shipping
enableCreateEventHandleAPIfeature used internally until we remove the flag.Also adds a bunch of test for this API with Activity and LegacyHidden, with inline gates for the differences with the flags.
Below is the claude summary.
Details
Fix Event Handle API blur events for Activity
Problem
The
beforeblurandafterblurevents from thecreateEventHandleAPI were not firing when a focused element was hidden using<React.Activity>. This was because the blur check incommitBeforeMutationEffectsOnFiberonly handledSuspenseComponent, notOffscreenComponent(which Activity uses internally).Solution
Added a check for
OffscreenComponentvisibility transitions behind a new feature flagenableEventAPIActivityFix.Files Modified
1.
packages/react-reconciler/src/ReactFiberCommitWork.jsAdded blur event handling for Activity (OffscreenComponent):
2. Feature Flag Files
Added
enableEventAPIActivityFixto all feature flag files:ReactFeatureFlags.jsfalse(default)ReactFeatureFlags.www-dynamic.js__VARIANT__(dynamic)ReactFeatureFlags.www.jsReactFeatureFlags.test-renderer.jsfalseReactFeatureFlags.test-renderer.www.jsfalseReactFeatureFlags.test-renderer.native-fb.jsfalse3.
packages/react-dom/src/events/__tests__/DOMPluginEventSystem-test.internal.jsAdded 22 new tests (from 120 → 142 total):
Activity tests (6 tests)
beforeblur and afterblur are called after a focused element is hiddenbeforeblur and afterblur are called after a nested focused element is hiddenbeforeblur should skip handlers from a hidden subtree after the focused element is hidden via ActivityLegacyHidden tests (6 tests)
beforeblur and afterblur are not called after a focused element is hidden inside LegacyHiddenbeforeblur and afterblur are not called after a nested focused element is hidden inside LegacyHiddenbeforeblur is not called after the focused element is hidden inside LegacyHiddenNested Activity + LegacyHidden tests (10 tests)
<Activity><LegacyHidden><Activity><LegacyHidden><Activity><LegacyHidden><Activity><LegacyHidden><Activity><LegacyHidden><LegacyHidden><Activity><LegacyHidden><Activity><LegacyHidden><Activity><LegacyHidden><Activity><LegacyHidden><Activity>Key Design Decisions
Activity triggers blur, LegacyHidden does not - LegacyHidden behavior remains unchanged for backwards compatibility
Inline gate pattern - Tests use
if(gate('enableEventAPIActivityFix'))to test both flag states in a single testWorks with enableViewTransition - The fix correctly handles the ViewTransition early exit path
Test Results
variant=true(enableEventAPIActivityFix ON)variant=false(enableEventAPIActivityFix OFF)enableViewTransition=true