Skip to content

Enable modern event system and delete dead code #19230

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.

Sign up for GitHub

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

Merged
gaearon merged 1 commit into from
Jul 1, 2020
Merged

Enable modern event system and delete dead code #19230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 1 addition & 4 deletions packages/react-dom/src/__tests__/ReactDOM-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,10 +357,7 @@ describe('ReactDOM', () => {
ReactDOM.render(<Wrapper />, container);
let expected;

if (
ReactFeatureFlags.enableModernEventSystem &
ReactFeatureFlags.enableLegacyFBSupport
) {
if (ReactFeatureFlags.enableLegacyFBSupport) {
// We expect to duplicate the 2nd handler because this test is
// not really designed around how the legacy FB support system works.
// This is because the above test sync fires a click() event
Expand Down
5 changes: 1 addition & 4 deletions packages/react-dom/src/__tests__/ReactDOMComponent-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2719,10 +2719,7 @@ describe('ReactDOMComponent', () => {
// might depend on this.
//
// @see https://github.com/facebook/react/pull/12919#issuecomment-395224674
if (
ReactFeatureFlags.enableModernEventSystem &
ReactFeatureFlags.enableLegacyFBSupport
) {
if (ReactFeatureFlags.enableLegacyFBSupport) {
// The order will change here, as the legacy FB support adds
// the event listener onto the document after the one above has.
expect(eventOrder).toEqual([
Expand Down
89 changes: 22 additions & 67 deletions packages/react-dom/src/__tests__/ReactDOMEventListener-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,42 +12,13 @@
describe('ReactDOMEventListener', () => {
let React;
let ReactDOM;
const ReactFeatureFlags = require('shared/ReactFeatureFlags');

beforeEach(() => {
jest.resetModules();
React = require('react');
ReactDOM = require('react-dom');
});

// We attached events to roots with the modern system,
// so this test is no longer valid.
if (!ReactFeatureFlags.enableModernEventSystem) {
it('should dispatch events from outside React tree', () => {
const mock = jest.fn();

const container = document.createElement('div');
const node = ReactDOM.render(<div onMouseEnter={mock} />, container);
const otherNode = document.createElement('h1');
document.body.appendChild(container);
document.body.appendChild(otherNode);

try {
otherNode.dispatchEvent(
new MouseEvent('mouseout', {
bubbles: true,
cancelable: true,
relatedTarget: node,
}),
);
expect(mock).toBeCalled();
} finally {
document.body.removeChild(container);
document.body.removeChild(otherNode);
}
});
}

describe('Propagation', () => {
it('should propagate events one level down', () => {
const mouseOut = jest.fn();
Expand Down Expand Up @@ -194,25 +165,19 @@ describe('ReactDOMEventListener', () => {
// The first call schedules a render of '1' into the 'Child'.
// However, we're batching so it isn't flushed yet.
expect(mock.mock.calls[0][0]).toBe('Child');
if (ReactFeatureFlags.enableModernEventSystem) {
// As we have two roots, it means we have two event listeners.
// This also means we enter the event batching phase twice,
// flushing the child to be 1.

// We don't have any good way of knowing if another event will
// occur because another event handler might invoke
// stopPropagation() along the way. After discussions internally
// with Sebastian, it seems that for now over-flushing should
// be fine, especially as the new event system is a breaking
// change anyway. We can maybe revisit this later as part of
// the work to refine this in the scheduler (maybe by leveraging
// isInputPending?).
expect(mock.mock.calls[1][0]).toBe('1');
} else {
// The first call schedules a render of '2' into the 'Child'.
// We're still batching so it isn't flushed yet either.
expect(mock.mock.calls[1][0]).toBe('Child');
}
// As we have two roots, it means we have two event listeners.
// This also means we enter the event batching phase twice,
// flushing the child to be 1.

// We don't have any good way of knowing if another event will
// occur because another event handler might invoke
// stopPropagation() along the way. After discussions internally
// with Sebastian, it seems that for now over-flushing should
// be fine, especially as the new event system is a breaking
// change anyway. We can maybe revisit this later as part of
// the work to refine this in the scheduler (maybe by leveraging
// isInputPending?).
expect(mock.mock.calls[1][0]).toBe('1');
// By the time we leave the handler, the second update is flushed.
expect(childNode.textContent).toBe('2');
} finally {
Expand Down Expand Up @@ -383,25 +348,15 @@ describe('ReactDOMEventListener', () => {
bubbles: false,
}),
);
if (ReactFeatureFlags.enableModernEventSystem) {
// As of the modern event system refactor, we now support
// this on <img>. The reason for this, is because we now
// attach all media events to the "root" or "portal" in the
// capture phase, rather than the bubble phase. This allows
// us to assign less event listeners to individual elements,
// which also nicely allows us to support more without needing
// to add more individual code paths to support various
// events that do not bubble.
expect(handleImgLoadStart).toHaveBeenCalledTimes(1);
} else {
// Historically, we happened to not support onLoadStart
// on <img>, and this test documents that lack of support.
// If we decide to support it in the future, we should change
// this line to expect 1 call. Note that fixing this would
// be simple but would require attaching a handler to each
// <img>. So far nobody asked us for it.
expect(handleImgLoadStart).toHaveBeenCalledTimes(0);
}
// As of the modern event system refactor, we now support
// this on <img>. The reason for this, is because we now
// attach all media events to the "root" or "portal" in the
// capture phase, rather than the bubble phase. This allows
// us to assign less event listeners to individual elements,
// which also nicely allows us to support more without needing
// to add more individual code paths to support various
// events that do not bubble.
expect(handleImgLoadStart).toHaveBeenCalledTimes(1);

videoRef.current.dispatchEvent(
new ProgressEvent('loadstart', {
Expand Down
94 changes: 0 additions & 94 deletions packages/react-dom/src/__tests__/ReactTestUtils-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ import * as ReactDOM from 'react-dom';
import * as ReactDOMServer from 'react-dom/server';
import * as ReactTestUtils from 'react-dom/test-utils';

const ReactFeatureFlags = require('shared/ReactFeatureFlags');

function getTestDocument(markup) {
const doc = document.implementation.createHTMLDocument('');
doc.open();
Expand All @@ -33,98 +31,6 @@ describe('ReactTestUtils', () => {
expect(Object.keys(ReactTestUtils.Simulate).sort()).toMatchSnapshot();
});

if (!ReactFeatureFlags.enableModernEventSystem) {
// SimulateNative API has been removed in the modern event system
it('SimulateNative should have locally attached media events', () => {
expect(Object.keys(ReactTestUtils.SimulateNative).sort()).toEqual([
'abort',
'animationEnd',
'animationIteration',
'animationStart',
'blur',
'canPlay',
'canPlayThrough',
'cancel',
'change',
'click',
'close',
'compositionEnd',
'compositionStart',
'compositionUpdate',
'contextMenu',
'copy',
'cut',
'doubleClick',
'drag',
'dragEnd',
'dragEnter',
'dragExit',
'dragLeave',
'dragOver',
'dragStart',
'drop',
'durationChange',
'emptied',
'encrypted',
'ended',
'error',
'focus',
'input',
'keyDown',
'keyPress',
'keyUp',
'load',
'loadStart',
'loadedData',
'loadedMetadata',
'mouseDown',
'mouseMove',
'mouseOut',
'mouseOver',
'mouseUp',
'paste',
'pause',
'play',
'playing',
'progress',
'rateChange',
'scroll',
'seeked',
'seeking',
'selectionChange',
'stalled',
'suspend',
'textInput',
'timeUpdate',
'toggle',
'touchCancel',
'touchEnd',
'touchMove',
'touchStart',
'transitionEnd',
'volumeChange',
'waiting',
'wheel',
]);
});

it('SimulateNative should warn about deprecation', () => {
const container = document.createElement('div');
const node = ReactDOM.render(<div />, container);
expect(() =>
ReactTestUtils.SimulateNative.click(node),
).toWarnDev(
'ReactTestUtils.SimulateNative is an undocumented API that does not match ' +
'how the browser dispatches events, and will be removed in a future major ' +
'version of React. If you rely on it for testing, consider attaching the root ' +
'DOM container to the document during the test, and then dispatching native browser ' +
'events by calling `node.dispatchEvent()` on the DOM nodes. Make sure to set ' +
'the `bubbles` flag to `true` when creating the native browser event.',
{withoutStack: true},
);
});
}

it('gives Jest mocks a passthrough implementation with mockComponent()', () => {
class MockedComponent extends React.Component {
render() {
Expand Down
120 changes: 37 additions & 83 deletions packages/react-dom/src/__tests__/ReactTreeTraversal-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

let React;
let ReactDOM;
const ReactFeatureFlags = require('shared/ReactFeatureFlags');

const ChildComponent = ({id, eventHandler}) => (
<div
Expand Down Expand Up @@ -204,89 +203,44 @@ describe('ReactTreeTraversal', () => {
expect(mockFn.mock.calls).toEqual(expectedCalls);
});

// This will not work with the modern event system that
// attaches event listeners to roots as the event below
// is being triggered on a node that React does not listen
// The modern event system attaches event listeners to roots so the
// event below is being triggered on a node that React does not listen
// to any more. Instead we should fire mouseover.
if (ReactFeatureFlags.enableModernEventSystem) {
it('should enter from the window', () => {
const enterNode = document.getElementById('P_P1_C1__DIV');

const expectedCalls = [
['P', 'mouseenter'],
['P_P1', 'mouseenter'],
['P_P1_C1__DIV', 'mouseenter'],
];

enterNode.dispatchEvent(
new MouseEvent('mouseover', {
bubbles: true,
cancelable: true,
relatedTarget: outerNode1,
}),
);

expect(mockFn.mock.calls).toEqual(expectedCalls);
});
} else {
it('should enter from the window', () => {
const enterNode = document.getElementById('P_P1_C1__DIV');

const expectedCalls = [
['P', 'mouseenter'],
['P_P1', 'mouseenter'],
['P_P1_C1__DIV', 'mouseenter'],
];

outerNode1.dispatchEvent(
new MouseEvent('mouseout', {
bubbles: true,
cancelable: true,
relatedTarget: enterNode,
}),
);

expect(mockFn.mock.calls).toEqual(expectedCalls);
});
}

// This will not work with the modern event system that
// attaches event listeners to roots as the event below
// is being triggered on a node that React does not listen
// to any more. Instead we should fire mouseover.
if (ReactFeatureFlags.enableModernEventSystem) {
it('should enter from the window to the shallowest', () => {
const enterNode = document.getElementById('P');

const expectedCalls = [['P', 'mouseenter']];

enterNode.dispatchEvent(
new MouseEvent('mouseover', {
bubbles: true,
cancelable: true,
relatedTarget: outerNode1,
}),
);

expect(mockFn.mock.calls).toEqual(expectedCalls);
});
} else {
it('should enter from the window to the shallowest', () => {
const enterNode = document.getElementById('P');

const expectedCalls = [['P', 'mouseenter']];

outerNode1.dispatchEvent(
new MouseEvent('mouseout', {
bubbles: true,
cancelable: true,
relatedTarget: enterNode,
}),
);

expect(mockFn.mock.calls).toEqual(expectedCalls);
});
}
it('should enter from the window', () => {
const enterNode = document.getElementById('P_P1_C1__DIV');

const expectedCalls = [
['P', 'mouseenter'],
['P_P1', 'mouseenter'],
['P_P1_C1__DIV', 'mouseenter'],
];

enterNode.dispatchEvent(
new MouseEvent('mouseover', {
bubbles: true,
cancelable: true,
relatedTarget: outerNode1,
}),
);

expect(mockFn.mock.calls).toEqual(expectedCalls);
});

it('should enter from the window to the shallowest', () => {
const enterNode = document.getElementById('P');

const expectedCalls = [['P', 'mouseenter']];

enterNode.dispatchEvent(
new MouseEvent('mouseover', {
bubbles: true,
cancelable: true,
relatedTarget: outerNode1,
}),
);

expect(mockFn.mock.calls).toEqual(expectedCalls);
});

it('should leave to the window', () => {
const leaveNode = document.getElementById('P_P1_C1__DIV');
Expand Down
Loading