{"parents":["b5eabd543f4a6c9d950f1bb5ec73da5a1c7c5caa"],"commit":{"authorEmail":"trueadm@users.noreply.github.com","shortRevision":"d95c4938df","authorName":"Dominic Gannaway","revision":"d95c4938df670a8f0a13267bd89173737bb185e4","authoredAt":"2020-10-14T23:38:57+01:00","subject":"[EventSystem] Revise onBeforeBlur propagation mechanics (#20020)"},"patch":"diff --git a\/packages\/react-art\/src\/ReactARTHostConfig.js b\/packages\/react-art\/src\/ReactARTHostConfig.js\nindex 459f714b90..43a563e90a 100644\n--- a\/packages\/react-art\/src\/ReactARTHostConfig.js\n+++ b\/packages\/react-art\/src\/ReactARTHostConfig.js\n@@ -467,7 +467,7 @@ export function makeClientIdInDEV(warnOnAccessInDEV: () => void): OpaqueIDType {\n throw new Error('Not yet implemented');\n }\n \n-export function beforeActiveInstanceBlur() {\n+export function beforeActiveInstanceBlur(internalInstanceHandle: Object) {\n \/\/ noop\n }\n \ndiff --git a\/packages\/react-dom\/src\/client\/ReactDOMHostConfig.js b\/packages\/react-dom\/src\/client\/ReactDOMHostConfig.js\nindex 63fdb08a3a..8de2f2c5c7 100644\n--- a\/packages\/react-dom\/src\/client\/ReactDOMHostConfig.js\n+++ b\/packages\/react-dom\/src\/client\/ReactDOMHostConfig.js\n@@ -232,10 +232,13 @@ export function prepareForCommit(containerInfo: Container): Object | null {\n return activeInstance;\n }\n \n-export function beforeActiveInstanceBlur(): void {\n+export function beforeActiveInstanceBlur(internalInstanceHandle: Object): void {\n if (enableCreateEventHandleAPI) {\n ReactBrowserEventEmitterSetEnabled(true);\n- dispatchBeforeDetachedBlur((selectionInformation: any).focusedElem);\n+ dispatchBeforeDetachedBlur(\n+ (selectionInformation: any).focusedElem,\n+ internalInstanceHandle,\n+ );\n ReactBrowserEventEmitterSetEnabled(false);\n }\n }\n@@ -499,12 +502,17 @@ function createEvent(type: DOMEventName, bubbles: boolean): Event {\n return event;\n }\n \n-function dispatchBeforeDetachedBlur(target: HTMLElement): void {\n+function dispatchBeforeDetachedBlur(\n+ target: HTMLElement,\n+ internalInstanceHandle: Object,\n+): void {\n if (enableCreateEventHandleAPI) {\n const event = createEvent('beforeblur', true);\n \/\/ Dispatch \"beforeblur\" directly on the target,\n \/\/ so it gets picked up by the event system and\n \/\/ can propagate through the React internal tree.\n+ \/\/ $FlowFixMe: internal field\n+ event._detachedInterceptFiber = internalInstanceHandle;\n target.dispatchEvent(event);\n }\n }\ndiff --git a\/packages\/react-dom\/src\/events\/DOMPluginEventSystem.js b\/packages\/react-dom\/src\/events\/DOMPluginEventSystem.js\nindex 74b95dea3d..c43d96dcb1 100644\n--- a\/packages\/react-dom\/src\/events\/DOMPluginEventSystem.js\n+++ b\/packages\/react-dom\/src\/events\/DOMPluginEventSystem.js\n@@ -658,10 +658,11 @@ export function accumulateSinglePhaseListeners(\n nativeEventType: string,\n inCapturePhase: boolean,\n accumulateTargetOnly: boolean,\n+ nativeEvent: AnyNativeEvent,\n ): Array {\n const captureName = reactName !== null ? reactName + 'Capture' : null;\n const reactEventName = inCapturePhase ? captureName : reactName;\n- const listeners: Array = [];\n+ let listeners: Array = [];\n \n let instance = targetFiber;\n let lastHostComponent = null;\n@@ -740,6 +741,23 @@ export function accumulateSinglePhaseListeners(\n if (accumulateTargetOnly) {\n break;\n }\n+ \/\/ If we are processing the onBeforeBlur event, then we need to take\n+ \/\/ into consideration that part of the React tree might have been hidden\n+ \/\/ or deleted (as we're invoking this event during commit). We can find\n+ \/\/ this out by checking if intercept fiber set on the event matches the\n+ \/\/ current instance fiber. In which case, we should clear all existing\n+ \/\/ listeners.\n+ if (enableCreateEventHandleAPI && nativeEvent.type === 'beforeblur') {\n+ \/\/ $FlowFixMe: internal field\n+ const detachedInterceptFiber = nativeEvent._detachedInterceptFiber;\n+ if (\n+ detachedInterceptFiber !== null &&\n+ (detachedInterceptFiber === instance ||\n+ detachedInterceptFiber === instance.alternate)\n+ ) {\n+ listeners = [];\n+ }\n+ }\n instance = instance.return;\n }\n return listeners;\ndiff --git a\/packages\/react-dom\/src\/events\/__tests__\/DOMPluginEventSystem-test.internal.js b\/packages\/react-dom\/src\/events\/__tests__\/DOMPluginEventSystem-test.internal.js\nindex 6a3662962a..004f0bb631 100644\n--- a\/packages\/react-dom\/src\/events\/__tests__\/DOMPluginEventSystem-test.internal.js\n+++ b\/packages\/react-dom\/src\/events\/__tests__\/DOMPluginEventSystem-test.internal.js\n@@ -2424,6 +2424,60 @@ describe('DOMPluginEventSystem', () => {\n expect(log).toEqual(['beforeblur', 'afterblur']);\n });\n \n+ \/\/ @gate experimental\n+ it('beforeblur should skip handlers from a deleted subtree after the focused element is unmounted', () => {\n+ const onBeforeBlur = jest.fn();\n+ const innerRef = React.createRef();\n+ const innerRef2 = React.createRef();\n+ const setBeforeBlurHandle = ReactDOM.unstable_createEventHandle(\n+ 'beforeblur',\n+ );\n+ const ref2 = React.createRef();\n+\n+ const Component = ({show}) => {\n+ const ref = React.useRef(null);\n+\n+ React.useEffect(() => {\n+ const clear1 = setBeforeBlurHandle(ref.current, onBeforeBlur);\n+ let clear2;\n+ if (ref2.current) {\n+ clear2 = setBeforeBlurHandle(ref2.current, onBeforeBlur);\n+ }\n+\n+ return () => {\n+ clear1();\n+ if (clear2) {\n+ clear2();\n+ }\n+ };\n+ });\n+\n+ return (\n+

\n+ {show && (\n+

\n+ \n+ <\/div>\n+ )}\n+

\n+ <\/div>\n+ );\n+ };\n+\n+ ReactDOM.render(, container);\n+ Scheduler.unstable_flushAll();\n+\n+ const inner = innerRef.current;\n+ const target = createEventTarget(inner);\n+ target.focus();\n+ expect(onBeforeBlur).toHaveBeenCalledTimes(0);\n+\n+ ReactDOM.render(, container);\n+ Scheduler.unstable_flushAll();\n+\n+ expect(onBeforeBlur).toHaveBeenCalledTimes(1);\n+ });\n+\n \/\/ @gate experimental\n it('beforeblur and afterblur are called after a focused element is suspended', () => {\n const log = [];\n@@ -2510,6 +2564,87 @@ describe('DOMPluginEventSystem', () => {\n document.body.removeChild(container2);\n });\n \n+ \/\/ @gate experimental\n+ it('beforeblur should skip handlers from a deleted subtree after the focused element is suspended', () => {\n+ const onBeforeBlur = jest.fn();\n+ const innerRef = React.createRef();\n+ const innerRef2 = React.createRef();\n+ const setBeforeBlurHandle = ReactDOM.unstable_createEventHandle(\n+ 'beforeblur',\n+ );\n+ const ref2 = React.createRef();\n+ const Suspense = React.Suspense;\n+ let suspend = false;\n+ let resolve;\n+ const promise = new Promise(\n+ resolvePromise => (resolve = resolvePromise),\n+ );\n+\n+ function Child() {\n+ if (suspend) {\n+ throw promise;\n+ } else {\n+ return ;\n+ }\n+ }\n+\n+ const Component = () => {\n+ const ref = React.useRef(null);\n+\n+ React.useEffect(() => {\n+ const clear1 = setBeforeBlurHandle(ref.current, onBeforeBlur);\n+ let clear2;\n+ if (ref2.current) {\n+ clear2 = setBeforeBlurHandle(ref2.current, onBeforeBlur);\n+ }\n+\n+ return () => {\n+ clear1();\n+ if (clear2) {\n+ clear2();\n+ }\n+ };\n+ });\n+\n+ return (\n+

\n+ \n+

\n+ \n+ <\/div>\n+ <\/Suspense>\n+

\n+ <\/div>\n+ );\n+ };\n+\n+ const container2 = document.createElement('div');\n+ document.body.appendChild(container2);\n+\n+ const root = ReactDOM.createRoot(container2);\n+\n+ act(() => {\n+ root.render();\n+ });\n+ jest.runAllTimers();\n+\n+ const inner = innerRef.current;\n+ const target = createEventTarget(inner);\n+ target.focus();\n+ expect(onBeforeBlur).toHaveBeenCalledTimes(0);\n+\n+ suspend = true;\n+ act(() => {\n+ root.render();\n+ });\n+ jest.runAllTimers();\n+\n+ expect(onBeforeBlur).toHaveBeenCalledTimes(1);\n+ resolve();\n+\n+ document.body.removeChild(container2);\n+ });\n+\n \/\/ @gate experimental\n it('regression: does not fire beforeblur\/afterblur if target is already hidden', () => {\n const Suspense = React.Suspense;\ndiff --git a\/packages\/react-dom\/src\/events\/plugins\/SimpleEventPlugin.js b\/packages\/react-dom\/src\/events\/plugins\/SimpleEventPlugin.js\nindex fafccff920..03bf9484b2 100644\n--- a\/packages\/react-dom\/src\/events\/plugins\/SimpleEventPlugin.js\n+++ b\/packages\/react-dom\/src\/events\/plugins\/SimpleEventPlugin.js\n@@ -200,6 +200,7 @@ function extractEvents(\n nativeEvent.type,\n inCapturePhase,\n accumulateTargetOnly,\n+ nativeEvent,\n );\n if (listeners.length > 0) {\n \/\/ Intentionally create event lazily.\ndiff --git a\/packages\/react-native-renderer\/src\/ReactFabricHostConfig.js b\/packages\/react-native-renderer\/src\/ReactFabricHostConfig.js\nindex dcdceb7a60..636542e8d4 100644\n--- a\/packages\/react-native-renderer\/src\/ReactFabricHostConfig.js\n+++ b\/packages\/react-native-renderer\/src\/ReactFabricHostConfig.js\n@@ -481,7 +481,7 @@ export function makeClientIdInDEV(warnOnAccessInDEV: () => void): OpaqueIDType {\n throw new Error('Not yet implemented');\n }\n \n-export function beforeActiveInstanceBlur() {\n+export function beforeActiveInstanceBlur(internalInstanceHandle: Object) {\n \/\/ noop\n }\n \ndiff --git a\/packages\/react-native-renderer\/src\/ReactNativeHostConfig.js b\/packages\/react-native-renderer\/src\/ReactNativeHostConfig.js\nindex 0369755c13..eba0a2b904 100644\n--- a\/packages\/react-native-renderer\/src\/ReactNativeHostConfig.js\n+++ b\/packages\/react-native-renderer\/src\/ReactNativeHostConfig.js\n@@ -534,7 +534,7 @@ export function makeClientIdInDEV(warnOnAccessInDEV: () => void): OpaqueIDType {\n throw new Error('Not yet implemented');\n }\n \n-export function beforeActiveInstanceBlur() {\n+export function beforeActiveInstanceBlur(internalInstanceHandle: Object) {\n \/\/ noop\n }\n \ndiff --git a\/packages\/react-reconciler\/src\/ReactFiberWorkLoop.new.js b\/packages\/react-reconciler\/src\/ReactFiberWorkLoop.new.js\nindex 4f40bde182..fffa71d208 100644\n--- a\/packages\/react-reconciler\/src\/ReactFiberWorkLoop.new.js\n+++ b\/packages\/react-reconciler\/src\/ReactFiberWorkLoop.new.js\n@@ -2172,7 +2172,7 @@ function commitBeforeMutationEffectsImpl(fiber: Fiber) {\n doesFiberContain(fiber, focusedInstanceHandle)\n ) {\n shouldFireAfterActiveInstanceBlur = true;\n- beforeActiveInstanceBlur();\n+ beforeActiveInstanceBlur(fiber);\n }\n }\n \n@@ -2206,7 +2206,7 @@ function commitBeforeMutationEffectsDeletions(deletions: Array) {\n \n if (doesFiberContain(fiber, ((focusedInstanceHandle: any): Fiber))) {\n shouldFireAfterActiveInstanceBlur = true;\n- beforeActiveInstanceBlur();\n+ beforeActiveInstanceBlur(fiber);\n }\n }\n }\ndiff --git a\/packages\/react-reconciler\/src\/ReactFiberWorkLoop.old.js b\/packages\/react-reconciler\/src\/ReactFiberWorkLoop.old.js\nindex c0fcb9c5c0..cece268e3d 100644\n--- a\/packages\/react-reconciler\/src\/ReactFiberWorkLoop.old.js\n+++ b\/packages\/react-reconciler\/src\/ReactFiberWorkLoop.old.js\n@@ -2261,7 +2261,7 @@ function commitBeforeMutationEffects() {\n if ((nextEffect.flags & Deletion) !== NoFlags) {\n if (doesFiberContain(nextEffect, focusedInstanceHandle)) {\n shouldFireAfterActiveInstanceBlur = true;\n- beforeActiveInstanceBlur();\n+ beforeActiveInstanceBlur(nextEffect);\n }\n } else {\n \/\/ TODO: Move this out of the hot path using a dedicated effect tag.\n@@ -2271,7 +2271,7 @@ function commitBeforeMutationEffects() {\n doesFiberContain(nextEffect, focusedInstanceHandle)\n ) {\n shouldFireAfterActiveInstanceBlur = true;\n- beforeActiveInstanceBlur();\n+ beforeActiveInstanceBlur(nextEffect);\n }\n }\n }\ndiff --git a\/packages\/react-test-renderer\/src\/ReactTestHostConfig.js b\/packages\/react-test-renderer\/src\/ReactTestHostConfig.js\nindex b244810736..a6d661d5fe 100644\n--- a\/packages\/react-test-renderer\/src\/ReactTestHostConfig.js\n+++ b\/packages\/react-test-renderer\/src\/ReactTestHostConfig.js\n@@ -377,7 +377,7 @@ export function makeOpaqueHydratingObject(\n };\n }\n \n-export function beforeActiveInstanceBlur() {\n+export function beforeActiveInstanceBlur(internalInstanceHandle: Object) {\n \/\/ noop\n }\n \n","body":"","files":[{"status":"M","path":"packages\/react-art\/src\/ReactARTHostConfig.js"},{"status":"M","path":"packages\/react-dom\/src\/client\/ReactDOMHostConfig.js"},{"status":"M","path":"packages\/react-dom\/src\/events\/DOMPluginEventSystem.js"},{"status":"M","path":"packages\/react-dom\/src\/events\/__tests__\/DOMPluginEventSystem-test.internal.js"},{"status":"M","path":"packages\/react-dom\/src\/events\/plugins\/SimpleEventPlugin.js"},{"status":"M","path":"packages\/react-native-renderer\/src\/ReactFabricHostConfig.js"},{"status":"M","path":"packages\/react-native-renderer\/src\/ReactNativeHostConfig.js"},{"status":"M","path":"packages\/react-reconciler\/src\/ReactFiberWorkLoop.new.js"},{"status":"M","path":"packages\/react-reconciler\/src\/ReactFiberWorkLoop.old.js"},{"status":"M","path":"packages\/react-test-renderer\/src\/ReactTestHostConfig.js"}]}

dillon1000/react

Files changed