|
12 | 12 | describe('ReactDOMEventListener', () => { |
13 | 13 | let React; |
14 | 14 | let ReactDOM; |
| 15 | + let ReactFeatureFlags = require('shared/ReactFeatureFlags'); |
15 | 16 |
|
16 | 17 | beforeEach(() => { |
17 | 18 | jest.resetModules(); |
18 | 19 | React = require('react'); |
19 | 20 | ReactDOM = require('react-dom'); |
20 | 21 | }); |
21 | 22 |
|
22 | | - it('should dispatch events from outside React tree', () => { |
23 | | - const mock = jest.fn(); |
| 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(); |
24 | 28 |
|
25 | | - const container = document.createElement('div'); |
26 | | - const node = ReactDOM.render(<div onMouseEnter={mock} />, container); |
27 | | - const otherNode = document.createElement('h1'); |
28 | | - document.body.appendChild(container); |
29 | | - document.body.appendChild(otherNode); |
| 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); |
30 | 34 |
|
31 | | - try { |
32 | | - otherNode.dispatchEvent( |
33 | | - new MouseEvent('mouseout', { |
34 | | - bubbles: true, |
35 | | - cancelable: true, |
36 | | - relatedTarget: node, |
37 | | - }), |
38 | | - ); |
39 | | - expect(mock).toBeCalled(); |
40 | | - } finally { |
41 | | - document.body.removeChild(container); |
42 | | - document.body.removeChild(otherNode); |
43 | | - } |
44 | | - }); |
| 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 | + } |
45 | 50 |
|
46 | 51 | describe('Propagation', () => { |
47 | 52 | it('should propagate events one level down', () => { |
@@ -189,9 +194,25 @@ describe('ReactDOMEventListener', () => { |
189 | 194 | // The first call schedules a render of '1' into the 'Child'. |
190 | 195 | // However, we're batching so it isn't flushed yet. |
191 | 196 | expect(mock.mock.calls[0][0]).toBe('Child'); |
192 | | - // The first call schedules a render of '2' into the 'Child'. |
193 | | - // We're still batching so it isn't flushed yet either. |
194 | | - expect(mock.mock.calls[1][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 | + } |
195 | 216 | // By the time we leave the handler, the second update is flushed. |
196 | 217 | expect(childNode.textContent).toBe('2'); |
197 | 218 | } finally { |
@@ -362,13 +383,25 @@ describe('ReactDOMEventListener', () => { |
362 | 383 | bubbles: false, |
363 | 384 | }), |
364 | 385 | ); |
365 | | - // Historically, we happened to not support onLoadStart |
366 | | - // on <img>, and this test documents that lack of support. |
367 | | - // If we decide to support it in the future, we should change |
368 | | - // this line to expect 1 call. Note that fixing this would |
369 | | - // be simple but would require attaching a handler to each |
370 | | - // <img>. So far nobody asked us for it. |
371 | | - expect(handleImgLoadStart).toHaveBeenCalledTimes(0); |
| 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 | + } |
372 | 405 |
|
373 | 406 | videoRef.current.dispatchEvent( |
374 | 407 | new ProgressEvent('loadstart', { |
|
0 commit comments