Skip to content

Commit 9fba65e

Browse files
gaearongaearon
authored
Enable modern event system and delete dead code (facebook#19230)
1 parent e3f4eb7 commit 9fba65e

File tree

52 files changed

+205
-7155
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+205
-7155
lines changed

‎packages/react-dom/src/__tests__/ReactDOM-test.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -357,10 +357,7 @@ describe('ReactDOM', () => {
357357
ReactDOM.render(<Wrapper />, container);
358358
let expected;
359359

360-
if (
361-
ReactFeatureFlags.enableModernEventSystem &
362-
ReactFeatureFlags.enableLegacyFBSupport
363-
) {
360+
if (ReactFeatureFlags.enableLegacyFBSupport) {
364361
// We expect to duplicate the 2nd handler because this test is
365362
// not really designed around how the legacy FB support system works.
366363
// This is because the above test sync fires a click() event

‎packages/react-dom/src/__tests__/ReactDOMComponent-test.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2719,10 +2719,7 @@ describe('ReactDOMComponent', () => {
27192719
// might depend on this.
27202720
//
27212721
// @see https://github.com/facebook/react/pull/12919#issuecomment-395224674
2722-
if (
2723-
ReactFeatureFlags.enableModernEventSystem &
2724-
ReactFeatureFlags.enableLegacyFBSupport
2725-
) {
2722+
if (ReactFeatureFlags.enableLegacyFBSupport) {
27262723
// The order will change here, as the legacy FB support adds
27272724
// the event listener onto the document after the one above has.
27282725
expect(eventOrder).toEqual([

‎packages/react-dom/src/__tests__/ReactDOMEventListener-test.js

Lines changed: 22 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -12,42 +12,13 @@
1212
describe('ReactDOMEventListener', () => {
1313
let React;
1414
let ReactDOM;
15-
const ReactFeatureFlags = require('shared/ReactFeatureFlags');
1615

1716
beforeEach(() => {
1817
jest.resetModules();
1918
React = require('react');
2019
ReactDOM = require('react-dom');
2120
});
2221

23-
// We attached events to roots with the modern system,
24-
// so this test is no longer valid.
25-
if (!ReactFeatureFlags.enableModernEventSystem) {
26-
it('should dispatch events from outside React tree', () => {
27-
const mock = jest.fn();
28-
29-
const container = document.createElement('div');
30-
const node = ReactDOM.render(<div onMouseEnter={mock} />, container);
31-
const otherNode = document.createElement('h1');
32-
document.body.appendChild(container);
33-
document.body.appendChild(otherNode);
34-
35-
try {
36-
otherNode.dispatchEvent(
37-
new MouseEvent('mouseout', {
38-
bubbles: true,
39-
cancelable: true,
40-
relatedTarget: node,
41-
}),
42-
);
43-
expect(mock).toBeCalled();
44-
} finally {
45-
document.body.removeChild(container);
46-
document.body.removeChild(otherNode);
47-
}
48-
});
49-
}
50-
5122
describe('Propagation', () => {
5223
it('should propagate events one level down', () => {
5324
const mouseOut = jest.fn();
@@ -194,25 +165,19 @@ describe('ReactDOMEventListener', () => {
194165
// The first call schedules a render of '1' into the 'Child'.
195166
// However, we're batching so it isn't flushed yet.
196167
expect(mock.mock.calls[0][0]).toBe('Child');
197-
if (ReactFeatureFlags.enableModernEventSystem) {
198-
// As we have two roots, it means we have two event listeners.
199-
// This also means we enter the event batching phase twice,
200-
// flushing the child to be 1.
201-
202-
// We don't have any good way of knowing if another event will
203-
// occur because another event handler might invoke
204-
// stopPropagation() along the way. After discussions internally
205-
// with Sebastian, it seems that for now over-flushing should
206-
// be fine, especially as the new event system is a breaking
207-
// change anyway. We can maybe revisit this later as part of
208-
// the work to refine this in the scheduler (maybe by leveraging
209-
// isInputPending?).
210-
expect(mock.mock.calls[1][0]).toBe('1');
211-
} else {
212-
// The first call schedules a render of '2' into the 'Child'.
213-
// We're still batching so it isn't flushed yet either.
214-
expect(mock.mock.calls[1][0]).toBe('Child');
215-
}
168+
// As we have two roots, it means we have two event listeners.
169+
// This also means we enter the event batching phase twice,
170+
// flushing the child to be 1.
171+
172+
// We don't have any good way of knowing if another event will
173+
// occur because another event handler might invoke
174+
// stopPropagation() along the way. After discussions internally
175+
// with Sebastian, it seems that for now over-flushing should
176+
// be fine, especially as the new event system is a breaking
177+
// change anyway. We can maybe revisit this later as part of
178+
// the work to refine this in the scheduler (maybe by leveraging
179+
// isInputPending?).
180+
expect(mock.mock.calls[1][0]).toBe('1');
216181
// By the time we leave the handler, the second update is flushed.
217182
expect(childNode.textContent).toBe('2');
218183
} finally {
@@ -383,25 +348,15 @@ describe('ReactDOMEventListener', () => {
383348
bubbles: false,
384349
}),
385350
);
386-
if (ReactFeatureFlags.enableModernEventSystem) {
387-
// As of the modern event system refactor, we now support
388-
// this on <img>. The reason for this, is because we now
389-
// attach all media events to the "root" or "portal" in the
390-
// capture phase, rather than the bubble phase. This allows
391-
// us to assign less event listeners to individual elements,
392-
// which also nicely allows us to support more without needing
393-
// to add more individual code paths to support various
394-
// events that do not bubble.
395-
expect(handleImgLoadStart).toHaveBeenCalledTimes(1);
396-
} else {
397-
// Historically, we happened to not support onLoadStart
398-
// on <img>, and this test documents that lack of support.
399-
// If we decide to support it in the future, we should change
400-
// this line to expect 1 call. Note that fixing this would
401-
// be simple but would require attaching a handler to each
402-
// <img>. So far nobody asked us for it.
403-
expect(handleImgLoadStart).toHaveBeenCalledTimes(0);
404-
}
351+
// As of the modern event system refactor, we now support
352+
// this on <img>. The reason for this, is because we now
353+
// attach all media events to the "root" or "portal" in the
354+
// capture phase, rather than the bubble phase. This allows
355+
// us to assign less event listeners to individual elements,
356+
// which also nicely allows us to support more without needing
357+
// to add more individual code paths to support various
358+
// events that do not bubble.
359+
expect(handleImgLoadStart).toHaveBeenCalledTimes(1);
405360

406361
videoRef.current.dispatchEvent(
407362
new ProgressEvent('loadstart', {

‎packages/react-dom/src/__tests__/ReactTestUtils-test.js

Lines changed: 0 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@ import * as ReactDOM from 'react-dom';
1515
import * as ReactDOMServer from 'react-dom/server';
1616
import * as ReactTestUtils from 'react-dom/test-utils';
1717

18-
const ReactFeatureFlags = require('shared/ReactFeatureFlags');
19-
2018
function getTestDocument(markup) {
2119
const doc = document.implementation.createHTMLDocument('');
2220
doc.open();
@@ -33,98 +31,6 @@ describe('ReactTestUtils', () => {
3331
expect(Object.keys(ReactTestUtils.Simulate).sort()).toMatchSnapshot();
3432
});
3533

36-
if (!ReactFeatureFlags.enableModernEventSystem) {
37-
// SimulateNative API has been removed in the modern event system
38-
it('SimulateNative should have locally attached media events', () => {
39-
expect(Object.keys(ReactTestUtils.SimulateNative).sort()).toEqual([
40-
'abort',
41-
'animationEnd',
42-
'animationIteration',
43-
'animationStart',
44-
'blur',
45-
'canPlay',
46-
'canPlayThrough',
47-
'cancel',
48-
'change',
49-
'click',
50-
'close',
51-
'compositionEnd',
52-
'compositionStart',
53-
'compositionUpdate',
54-
'contextMenu',
55-
'copy',
56-
'cut',
57-
'doubleClick',
58-
'drag',
59-
'dragEnd',
60-
'dragEnter',
61-
'dragExit',
62-
'dragLeave',
63-
'dragOver',
64-
'dragStart',
65-
'drop',
66-
'durationChange',
67-
'emptied',
68-
'encrypted',
69-
'ended',
70-
'error',
71-
'focus',
72-
'input',
73-
'keyDown',
74-
'keyPress',
75-
'keyUp',
76-
'load',
77-
'loadStart',
78-
'loadedData',
79-
'loadedMetadata',
80-
'mouseDown',
81-
'mouseMove',
82-
'mouseOut',
83-
'mouseOver',
84-
'mouseUp',
85-
'paste',
86-
'pause',
87-
'play',
88-
'playing',
89-
'progress',
90-
'rateChange',
91-
'scroll',
92-
'seeked',
93-
'seeking',
94-
'selectionChange',
95-
'stalled',
96-
'suspend',
97-
'textInput',
98-
'timeUpdate',
99-
'toggle',
100-
'touchCancel',
101-
'touchEnd',
102-
'touchMove',
103-
'touchStart',
104-
'transitionEnd',
105-
'volumeChange',
106-
'waiting',
107-
'wheel',
108-
]);
109-
});
110-
111-
it('SimulateNative should warn about deprecation', () => {
112-
const container = document.createElement('div');
113-
const node = ReactDOM.render(<div />, container);
114-
expect(() =>
115-
ReactTestUtils.SimulateNative.click(node),
116-
).toWarnDev(
117-
'ReactTestUtils.SimulateNative is an undocumented API that does not match ' +
118-
'how the browser dispatches events, and will be removed in a future major ' +
119-
'version of React. If you rely on it for testing, consider attaching the root ' +
120-
'DOM container to the document during the test, and then dispatching native browser ' +
121-
'events by calling `node.dispatchEvent()` on the DOM nodes. Make sure to set ' +
122-
'the `bubbles` flag to `true` when creating the native browser event.',
123-
{withoutStack: true},
124-
);
125-
});
126-
}
127-
12834
it('gives Jest mocks a passthrough implementation with mockComponent()', () => {
12935
class MockedComponent extends React.Component {
13036
render() {

‎packages/react-dom/src/__tests__/ReactTreeTraversal-test.js

Lines changed: 37 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111

1212
let React;
1313
let ReactDOM;
14-
const ReactFeatureFlags = require('shared/ReactFeatureFlags');
1514

1615
const ChildComponent = ({id, eventHandler}) => (
1716
<div
@@ -204,89 +203,44 @@ describe('ReactTreeTraversal', () => {
204203
expect(mockFn.mock.calls).toEqual(expectedCalls);
205204
});
206205

207-
// This will not work with the modern event system that
208-
// attaches event listeners to roots as the event below
209-
// is being triggered on a node that React does not listen
206+
// The modern event system attaches event listeners to roots so the
207+
// event below is being triggered on a node that React does not listen
210208
// to any more. Instead we should fire mouseover.
211-
if (ReactFeatureFlags.enableModernEventSystem) {
212-
it('should enter from the window', () => {
213-
const enterNode = document.getElementById('P_P1_C1__DIV');
214-
215-
const expectedCalls = [
216-
['P', 'mouseenter'],
217-
['P_P1', 'mouseenter'],
218-
['P_P1_C1__DIV', 'mouseenter'],
219-
];
220-
221-
enterNode.dispatchEvent(
222-
new MouseEvent('mouseover', {
223-
bubbles: true,
224-
cancelable: true,
225-
relatedTarget: outerNode1,
226-
}),
227-
);
228-
229-
expect(mockFn.mock.calls).toEqual(expectedCalls);
230-
});
231-
} else {
232-
it('should enter from the window', () => {
233-
const enterNode = document.getElementById('P_P1_C1__DIV');
234-
235-
const expectedCalls = [
236-
['P', 'mouseenter'],
237-
['P_P1', 'mouseenter'],
238-
['P_P1_C1__DIV', 'mouseenter'],
239-
];
240-
241-
outerNode1.dispatchEvent(
242-
new MouseEvent('mouseout', {
243-
bubbles: true,
244-
cancelable: true,
245-
relatedTarget: enterNode,
246-
}),
247-
);
248-
249-
expect(mockFn.mock.calls).toEqual(expectedCalls);
250-
});
251-
}
252-
253-
// This will not work with the modern event system that
254-
// attaches event listeners to roots as the event below
255-
// is being triggered on a node that React does not listen
256-
// to any more. Instead we should fire mouseover.
257-
if (ReactFeatureFlags.enableModernEventSystem) {
258-
it('should enter from the window to the shallowest', () => {
259-
const enterNode = document.getElementById('P');
260-
261-
const expectedCalls = [['P', 'mouseenter']];
262-
263-
enterNode.dispatchEvent(
264-
new MouseEvent('mouseover', {
265-
bubbles: true,
266-
cancelable: true,
267-
relatedTarget: outerNode1,
268-
}),
269-
);
270-
271-
expect(mockFn.mock.calls).toEqual(expectedCalls);
272-
});
273-
} else {
274-
it('should enter from the window to the shallowest', () => {
275-
const enterNode = document.getElementById('P');
276-
277-
const expectedCalls = [['P', 'mouseenter']];
278-
279-
outerNode1.dispatchEvent(
280-
new MouseEvent('mouseout', {
281-
bubbles: true,
282-
cancelable: true,
283-
relatedTarget: enterNode,
284-
}),
285-
);
286-
287-
expect(mockFn.mock.calls).toEqual(expectedCalls);
288-
});
289-
}
209+
it('should enter from the window', () => {
210+
const enterNode = document.getElementById('P_P1_C1__DIV');
211+
212+
const expectedCalls = [
213+
['P', 'mouseenter'],
214+
['P_P1', 'mouseenter'],
215+
['P_P1_C1__DIV', 'mouseenter'],
216+
];
217+
218+
enterNode.dispatchEvent(
219+
new MouseEvent('mouseover', {
220+
bubbles: true,
221+
cancelable: true,
222+
relatedTarget: outerNode1,
223+
}),
224+
);
225+
226+
expect(mockFn.mock.calls).toEqual(expectedCalls);
227+
});
228+
229+
it('should enter from the window to the shallowest', () => {
230+
const enterNode = document.getElementById('P');
231+
232+
const expectedCalls = [['P', 'mouseenter']];
233+
234+
enterNode.dispatchEvent(
235+
new MouseEvent('mouseover', {
236+
bubbles: true,
237+
cancelable: true,
238+
relatedTarget: outerNode1,
239+
}),
240+
);
241+
242+
expect(mockFn.mock.calls).toEqual(expectedCalls);
243+
});
290244

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

0 commit comments

Comments
 (0)