Skip to content

Commit

Permalink
[EventSystem] Revise onBeforeBlur propagation mechanics (facebook#20020)
Browse files Browse the repository at this point in the history
  • Loading branch information
trueadm authored and koto committed Jun 15, 2021
1 parent 3c12274 commit acba0bd
Show file tree
Hide file tree
Showing 10 changed files with 174 additions and 12 deletions.
2 changes: 1 addition & 1 deletion packages/react-art/src/ReactARTHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ export function makeClientIdInDEV(warnOnAccessInDEV: () => void): OpaqueIDType {
throw new Error('Not yet implemented');
}

export function beforeActiveInstanceBlur() {
export function beforeActiveInstanceBlur(internalInstanceHandle: Object) {
// noop
}

Expand Down
14 changes: 11 additions & 3 deletions packages/react-dom/src/client/ReactDOMHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -232,10 +232,13 @@ export function prepareForCommit(containerInfo: Container): Object | null {
return activeInstance;
}

export function beforeActiveInstanceBlur(): void {
export function beforeActiveInstanceBlur(internalInstanceHandle: Object): void {
if (enableCreateEventHandleAPI) {
ReactBrowserEventEmitterSetEnabled(true);
dispatchBeforeDetachedBlur((selectionInformation: any).focusedElem);
dispatchBeforeDetachedBlur(
(selectionInformation: any).focusedElem,
internalInstanceHandle,
);
ReactBrowserEventEmitterSetEnabled(false);
}
}
Expand Down Expand Up @@ -499,12 +502,17 @@ function createEvent(type: DOMEventName, bubbles: boolean): Event {
return event;
}

function dispatchBeforeDetachedBlur(target: HTMLElement): void {
function dispatchBeforeDetachedBlur(
target: HTMLElement,
internalInstanceHandle: Object,
): void {
if (enableCreateEventHandleAPI) {
const event = createEvent('beforeblur', true);
// Dispatch "beforeblur" directly on the target,
// so it gets picked up by the event system and
// can propagate through the React internal tree.
// $FlowFixMe: internal field
event._detachedInterceptFiber = internalInstanceHandle;
target.dispatchEvent(event);
}
}
Expand Down
20 changes: 19 additions & 1 deletion packages/react-dom/src/events/DOMPluginEventSystem.js
Original file line number Diff line number Diff line change
Expand Up @@ -658,10 +658,11 @@ export function accumulateSinglePhaseListeners(
nativeEventType: string,
inCapturePhase: boolean,
accumulateTargetOnly: boolean,
nativeEvent: AnyNativeEvent,
): Array<DispatchListener> {
const captureName = reactName !== null ? reactName + 'Capture' : null;
const reactEventName = inCapturePhase ? captureName : reactName;
const listeners: Array<DispatchListener> = [];
let listeners: Array<DispatchListener> = [];

let instance = targetFiber;
let lastHostComponent = null;
Expand Down Expand Up @@ -740,6 +741,23 @@ export function accumulateSinglePhaseListeners(
if (accumulateTargetOnly) {
break;
}
// If we are processing the onBeforeBlur event, then we need to take
// into consideration that part of the React tree might have been hidden
// or deleted (as we're invoking this event during commit). We can find
// this out by checking if intercept fiber set on the event matches the
// current instance fiber. In which case, we should clear all existing
// listeners.
if (enableCreateEventHandleAPI && nativeEvent.type === 'beforeblur') {
// $FlowFixMe: internal field
const detachedInterceptFiber = nativeEvent._detachedInterceptFiber;
if (
detachedInterceptFiber !== null &&
(detachedInterceptFiber === instance ||
detachedInterceptFiber === instance.alternate)
) {
listeners = [];
}
}
instance = instance.return;
}
return listeners;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2424,6 +2424,60 @@ describe('DOMPluginEventSystem', () => {
expect(log).toEqual(['beforeblur', 'afterblur']);
});

// @gate experimental
it('beforeblur should skip handlers from a deleted subtree after the focused element is unmounted', () => {
const onBeforeBlur = jest.fn();
const innerRef = React.createRef();
const innerRef2 = React.createRef();
const setBeforeBlurHandle = ReactDOM.unstable_createEventHandle(
'beforeblur',
);
const ref2 = React.createRef();

const Component = ({show}) => {
const ref = React.useRef(null);

React.useEffect(() => {
const clear1 = setBeforeBlurHandle(ref.current, onBeforeBlur);
let clear2;
if (ref2.current) {
clear2 = setBeforeBlurHandle(ref2.current, onBeforeBlur);
}

return () => {
clear1();
if (clear2) {
clear2();
}
};
});

return (
<div ref={ref}>
{show && (
<div ref={ref2}>
<input ref={innerRef} />
</div>
)}
<div ref={innerRef2} />
</div>
);
};

ReactDOM.render(<Component show={true} />, container);
Scheduler.unstable_flushAll();

const inner = innerRef.current;
const target = createEventTarget(inner);
target.focus();
expect(onBeforeBlur).toHaveBeenCalledTimes(0);

ReactDOM.render(<Component show={false} />, container);
Scheduler.unstable_flushAll();

expect(onBeforeBlur).toHaveBeenCalledTimes(1);
});

// @gate experimental
it('beforeblur and afterblur are called after a focused element is suspended', () => {
const log = [];
Expand Down Expand Up @@ -2510,6 +2564,87 @@ describe('DOMPluginEventSystem', () => {
document.body.removeChild(container2);
});

// @gate experimental
it('beforeblur should skip handlers from a deleted subtree after the focused element is suspended', () => {
const onBeforeBlur = jest.fn();
const innerRef = React.createRef();
const innerRef2 = React.createRef();
const setBeforeBlurHandle = ReactDOM.unstable_createEventHandle(
'beforeblur',
);
const ref2 = React.createRef();
const Suspense = React.Suspense;
let suspend = false;
let resolve;
const promise = new Promise(
resolvePromise => (resolve = resolvePromise),
);

function Child() {
if (suspend) {
throw promise;
} else {
return <input ref={innerRef} />;
}
}

const Component = () => {
const ref = React.useRef(null);

React.useEffect(() => {
const clear1 = setBeforeBlurHandle(ref.current, onBeforeBlur);
let clear2;
if (ref2.current) {
clear2 = setBeforeBlurHandle(ref2.current, onBeforeBlur);
}

return () => {
clear1();
if (clear2) {
clear2();
}
};
});

return (
<div ref={ref}>
<Suspense fallback="Loading...">
<div ref={ref2}>
<Child />
</div>
</Suspense>
<div ref={innerRef2} />
</div>
);
};

const container2 = document.createElement('div');
document.body.appendChild(container2);

const root = ReactDOM.createRoot(container2);

act(() => {
root.render(<Component />);
});
jest.runAllTimers();

const inner = innerRef.current;
const target = createEventTarget(inner);
target.focus();
expect(onBeforeBlur).toHaveBeenCalledTimes(0);

suspend = true;
act(() => {
root.render(<Component />);
});
jest.runAllTimers();

expect(onBeforeBlur).toHaveBeenCalledTimes(1);
resolve();

document.body.removeChild(container2);
});

// @gate experimental
it('regression: does not fire beforeblur/afterblur if target is already hidden', () => {
const Suspense = React.Suspense;
Expand Down
1 change: 1 addition & 0 deletions packages/react-dom/src/events/plugins/SimpleEventPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ function extractEvents(
nativeEvent.type,
inCapturePhase,
accumulateTargetOnly,
nativeEvent,
);
if (listeners.length > 0) {
// Intentionally create event lazily.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ export function makeClientIdInDEV(warnOnAccessInDEV: () => void): OpaqueIDType {
throw new Error('Not yet implemented');
}

export function beforeActiveInstanceBlur() {
export function beforeActiveInstanceBlur(internalInstanceHandle: Object) {
// noop
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ export function makeClientIdInDEV(warnOnAccessInDEV: () => void): OpaqueIDType {
throw new Error('Not yet implemented');
}

export function beforeActiveInstanceBlur() {
export function beforeActiveInstanceBlur(internalInstanceHandle: Object) {
// noop
}

Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -2172,7 +2172,7 @@ function commitBeforeMutationEffectsImpl(fiber: Fiber) {
doesFiberContain(fiber, focusedInstanceHandle)
) {
shouldFireAfterActiveInstanceBlur = true;
beforeActiveInstanceBlur();
beforeActiveInstanceBlur(fiber);
}
}

Expand Down Expand Up @@ -2206,7 +2206,7 @@ function commitBeforeMutationEffectsDeletions(deletions: Array<Fiber>) {

if (doesFiberContain(fiber, ((focusedInstanceHandle: any): Fiber))) {
shouldFireAfterActiveInstanceBlur = true;
beforeActiveInstanceBlur();
beforeActiveInstanceBlur(fiber);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -2261,7 +2261,7 @@ function commitBeforeMutationEffects() {
if ((nextEffect.flags & Deletion) !== NoFlags) {
if (doesFiberContain(nextEffect, focusedInstanceHandle)) {
shouldFireAfterActiveInstanceBlur = true;
beforeActiveInstanceBlur();
beforeActiveInstanceBlur(nextEffect);
}
} else {
// TODO: Move this out of the hot path using a dedicated effect tag.
Expand All @@ -2271,7 +2271,7 @@ function commitBeforeMutationEffects() {
doesFiberContain(nextEffect, focusedInstanceHandle)
) {
shouldFireAfterActiveInstanceBlur = true;
beforeActiveInstanceBlur();
beforeActiveInstanceBlur(nextEffect);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/react-test-renderer/src/ReactTestHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ export function makeOpaqueHydratingObject(
};
}

export function beforeActiveInstanceBlur() {
export function beforeActiveInstanceBlur(internalInstanceHandle: Object) {
// noop
}

Expand Down

0 comments on commit acba0bd

Please sign in to comment.