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

[EventSystem] Revise onBeforeBlur propagation mechanics #20020

Merged
merged 2 commits into from
Oct 14, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
17 changes: 16 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,20 @@ 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
nativeEvent._detachedInterceptFiber === instance
trueadm marked this conversation as resolved.
Show resolved Hide resolved
) {
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 has the correct propagation mechancis after a nested focused element is unmounted', () => {
trueadm marked this conversation as resolved.
Show resolved Hide resolved
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 has the correct propagation mechancis after a nested 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(current);
}
}

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(current);
}
}
}
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