From 32793fbacf826862cc9c12809943f863eb670938 Mon Sep 17 00:00:00 2001 From: Philipp Spiess Date: Thu, 9 Aug 2018 21:47:01 +0200 Subject: [PATCH 1/4] Update dom fixture lockfile --- fixtures/dom/yarn.lock | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/fixtures/dom/yarn.lock b/fixtures/dom/yarn.lock index 188f76fb40214..3ccd977580be8 100644 --- a/fixtures/dom/yarn.lock +++ b/fixtures/dom/yarn.lock @@ -2153,14 +2153,6 @@ dotenv@4.0.0: version "4.0.0" resolved "https://registry.yarnpkg.com/dotenv/-/dotenv-4.0.0.tgz#864ef1379aced55ce6f95debecdce179f7a0cd1d" -draft-js@^0.10.5: - version "0.10.5" - resolved "https://registry.yarnpkg.com/draft-js/-/draft-js-0.10.5.tgz#bfa9beb018fe0533dbb08d6675c371a6b08fa742" - dependencies: - fbjs "^0.8.15" - immutable "~3.7.4" - object-assign "^4.1.0" - duplexer2@^0.1.4: version "0.1.4" resolved "https://registry.yarnpkg.com/duplexer2/-/duplexer2-0.1.4.tgz#8b12dab878c0d69e3e7891051662a32fc6bddcc1" @@ -2680,7 +2672,7 @@ fbjs@^0.8.1, fbjs@^0.8.4: setimmediate "^1.0.5" ua-parser-js "^0.7.9" -fbjs@^0.8.15, fbjs@^0.8.16: +fbjs@^0.8.16: version "0.8.16" resolved "https://registry.yarnpkg.com/fbjs/-/fbjs-0.8.16.tgz#5e67432f550dc41b572bf55847b8aca64e5337db" dependencies: @@ -3310,10 +3302,6 @@ ignore@^3.3.3: version "3.3.3" resolved "https://registry.yarnpkg.com/ignore/-/ignore-3.3.3.tgz#432352e57accd87ab3110e82d3fea0e47812156d" -immutable@~3.7.4: - version "3.7.6" - resolved "https://registry.yarnpkg.com/immutable/-/immutable-3.7.6.tgz#13b4d3cb12befa15482a26fe1b2ebae640071e4b" - imurmurhash@^0.1.4: version "0.1.4" resolved "https://registry.yarnpkg.com/imurmurhash/-/imurmurhash-0.1.4.tgz#9218b9b2b928a238b13dc4fb6b6d576f231453ea" From d26313831e4744f552825748516cb7b1e684391f Mon Sep 17 00:00:00 2001 From: Philipp Spiess Date: Thu, 9 Aug 2018 20:32:13 +0200 Subject: [PATCH 2/4] Bring back onSubmit bubble test I found a test that was written more than 5 years ago and probably never run until now. The behavior still works, although the API changed quite a bit over the years. Seems like this was part of the initial public release already: https://github.com/facebook/react/commit/75897c2dcd1dd3a6ca46284dd37e13d22b4b16b4#diff-1bf5126edab96f3b7fea034cd3b0c742R31 --- .../react-dom/src/__tests__/ReactDOM-test.js | 59 +++++++++++-------- 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOM-test.js b/packages/react-dom/src/__tests__/ReactDOM-test.js index 08aff5b582695..d5212c86e28b9 100644 --- a/packages/react-dom/src/__tests__/ReactDOM-test.js +++ b/packages/react-dom/src/__tests__/ReactDOM-test.js @@ -23,34 +23,41 @@ describe('ReactDOM', () => { ReactTestUtils = require('react-dom/test-utils'); }); - // TODO: uncomment this test once we can run in phantom, which - // supports real submit events. - /* it('should bubble onSubmit', function() { - const count = 0; - const form; - const Parent = React.createClass({ - handleSubmit: function() { - count++; - return false; - }, - render: function() { - return ; - } - }); - const Child = React.createClass({ - render: function() { - return
; - }, - componentDidMount: function() { - form = ReactDOM.findDOMNode(this); - } - }); - const instance = ReactTestUtils.renderIntoDocument(); - form.submit(); - expect(count).toEqual(1); + const container = document.createElement('div'); + + let count = 0; + let buttonRef; + + function Parent() { + return ( +
{ + event.preventDefault(); + count++; + }}> + +
+ ); + } + + function Child() { + return ( +
+ (buttonRef = button)} /> +
+ ); + } + + document.body.appendChild(container); + try { + ReactDOM.render(, container); + buttonRef.click(); + expect(count).toBe(1); + } finally { + document.body.removeChild(container); + } }); - */ it('allows a DOM element to be used with a string', () => { const element = React.createElement('div', {className: 'foo'}); From 2f2258173a6a880c26bc7e4a3e3adc2b49d21b77 Mon Sep 17 00:00:00 2001 From: Philipp Spiess Date: Wed, 22 Aug 2018 21:14:58 +0200 Subject: [PATCH 3/4] Add test to make sure interim native events are prevented For more information, see #13462 --- .../__tests__/ReactDOMEventListener-test.js | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js index f34688661c391..703c23c2f1436 100644 --- a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js @@ -271,6 +271,51 @@ describe('ReactDOMEventListener', () => { document.body.removeChild(container); }); + // This is a special case for submit and reset events as they are listened on + // at the element level and not the document. + // @see https://github.com/facebook/react/pull/13462 + it('should not receive submit events if native, interim DOM handler prevents it', () => { + const container = document.createElement('div'); + document.body.appendChild(container); + + try { + const formRef = React.createRef(); + const interimRef = React.createRef(); + + const handleSubmit = jest.fn(); + const handleReset = jest.fn(); + ReactDOM.render( +
+
+
, + container, + ); + + interimRef.current.onsubmit = nativeEvent => + nativeEvent.stopPropagation(); + interimRef.current.onreset = nativeEvent => nativeEvent.stopPropagation(); + + formRef.current.dispatchEvent( + new Event('submit', { + // https://developer.mozilla.org/en-US/docs/Web/Events/submit + bubbles: true, + }), + ); + + formRef.current.dispatchEvent( + new Event('reset', { + // https://developer.mozilla.org/en-US/docs/Web/Events/reset + bubbles: true, + }), + ); + + expect(handleSubmit).toHaveBeenCalled(); + expect(handleReset).toHaveBeenCalled(); + } finally { + document.body.removeChild(container); + } + }); + it('should dispatch loadstart only for media elements', () => { const container = document.createElement('div'); document.body.appendChild(container); From 2a9335a1ab8b342e3312ca4b0df9c861650f3beb Mon Sep 17 00:00:00 2001 From: Philipp Spiess Date: Wed, 22 Aug 2018 21:25:28 +0200 Subject: [PATCH 4/4] Make sure all DOM mutations are cleaned up --- .../__tests__/ReactDOMEventListener-test.js | 239 ++++++++++-------- 1 file changed, 131 insertions(+), 108 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js index 703c23c2f1436..82f42641d1f1f 100644 --- a/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMEventListener-test.js @@ -28,14 +28,19 @@ describe('ReactDOMEventListener', () => { document.body.appendChild(container); document.body.appendChild(otherNode); - otherNode.dispatchEvent( - new MouseEvent('mouseout', { - bubbles: true, - cancelable: true, - relatedTarget: node, - }), - ); - expect(mock).toBeCalled(); + 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', () => { @@ -56,16 +61,18 @@ describe('ReactDOMEventListener', () => { parentNode.appendChild(childContainer); document.body.appendChild(parentContainer); - const nativeEvent = document.createEvent('Event'); - nativeEvent.initEvent('mouseout', true, true); - childNode.dispatchEvent(nativeEvent); - - expect(mouseOut).toBeCalled(); - expect(mouseOut).toHaveBeenCalledTimes(2); - expect(mouseOut.mock.calls[0][0]).toEqual(childNode); - expect(mouseOut.mock.calls[1][0]).toEqual(parentNode); - - document.body.removeChild(parentContainer); + try { + const nativeEvent = document.createEvent('Event'); + nativeEvent.initEvent('mouseout', true, true); + childNode.dispatchEvent(nativeEvent); + + expect(mouseOut).toBeCalled(); + expect(mouseOut).toHaveBeenCalledTimes(2); + expect(mouseOut.mock.calls[0][0]).toEqual(childNode); + expect(mouseOut.mock.calls[1][0]).toEqual(parentNode); + } finally { + document.body.removeChild(parentContainer); + } }); it('should propagate events two levels down', () => { @@ -92,50 +99,58 @@ describe('ReactDOMEventListener', () => { document.body.appendChild(grandParentContainer); - const nativeEvent = document.createEvent('Event'); - nativeEvent.initEvent('mouseout', true, true); - childNode.dispatchEvent(nativeEvent); - - expect(mouseOut).toBeCalled(); - expect(mouseOut).toHaveBeenCalledTimes(3); - expect(mouseOut.mock.calls[0][0]).toEqual(childNode); - expect(mouseOut.mock.calls[1][0]).toEqual(parentNode); - expect(mouseOut.mock.calls[2][0]).toEqual(grandParentNode); - - document.body.removeChild(grandParentContainer); + try { + const nativeEvent = document.createEvent('Event'); + nativeEvent.initEvent('mouseout', true, true); + childNode.dispatchEvent(nativeEvent); + + expect(mouseOut).toBeCalled(); + expect(mouseOut).toHaveBeenCalledTimes(3); + expect(mouseOut.mock.calls[0][0]).toEqual(childNode); + expect(mouseOut.mock.calls[1][0]).toEqual(parentNode); + expect(mouseOut.mock.calls[2][0]).toEqual(grandParentNode); + } finally { + document.body.removeChild(grandParentContainer); + } }); // Regression test for https://github.com/facebook/react/issues/1105 it('should not get confused by disappearing elements', () => { const container = document.createElement('div'); document.body.appendChild(container); - class MyComponent extends React.Component { - state = {clicked: false}; - handleClick = () => { - this.setState({clicked: true}); - }; - componentDidMount() { - expect(ReactDOM.findDOMNode(this)).toBe(container.firstChild); - } - componentDidUpdate() { - expect(ReactDOM.findDOMNode(this)).toBe(container.firstChild); - } - render() { - if (this.state.clicked) { - return clicked!; - } else { - return ; + + try { + class MyComponent extends React.Component { + state = {clicked: false}; + handleClick = () => { + this.setState({clicked: true}); + }; + componentDidMount() { + expect(ReactDOM.findDOMNode(this)).toBe(container.firstChild); + } + componentDidUpdate() { + expect(ReactDOM.findDOMNode(this)).toBe(container.firstChild); + } + render() { + if (this.state.clicked) { + return clicked!; + } else { + return ( + + ); + } } } + ReactDOM.render(, container); + container.firstChild.dispatchEvent( + new MouseEvent('click', { + bubbles: true, + }), + ); + expect(container.firstChild.textContent).toBe('clicked!'); + } finally { + document.body.removeChild(container); } - ReactDOM.render(, container); - container.firstChild.dispatchEvent( - new MouseEvent('click', { - bubbles: true, - }), - ); - expect(container.firstChild.textContent).toBe('clicked!'); - document.body.removeChild(container); }); it('should batch between handlers from different roots', () => { @@ -164,21 +179,24 @@ describe('ReactDOMEventListener', () => { parentNode.appendChild(childContainer); document.body.appendChild(parentContainer); - const nativeEvent = document.createEvent('Event'); - nativeEvent.initEvent('mouseout', true, true); - childNode.dispatchEvent(nativeEvent); - - // Child and parent should both call from event handlers. - expect(mock).toHaveBeenCalledTimes(2); - // 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'); - // 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'); - // By the time we leave the handler, the second update is flushed. - expect(childNode.textContent).toBe('2'); - document.body.removeChild(parentContainer); + try { + const nativeEvent = document.createEvent('Event'); + nativeEvent.initEvent('mouseout', true, true); + childNode.dispatchEvent(nativeEvent); + + // Child and parent should both call from event handlers. + expect(mock).toHaveBeenCalledTimes(2); + // 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'); + // 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'); + // By the time we leave the handler, the second update is flushed. + expect(childNode.textContent).toBe('2'); + } finally { + document.body.removeChild(parentContainer); + } }); }); @@ -208,14 +226,17 @@ describe('ReactDOMEventListener', () => { document.body.appendChild(container); - const nativeEvent = document.createEvent('Event'); - nativeEvent.initEvent('mouseout', true, true); - instance.getInner().dispatchEvent(nativeEvent); + try { + const nativeEvent = document.createEvent('Event'); + nativeEvent.initEvent('mouseout', true, true); + instance.getInner().dispatchEvent(nativeEvent); - expect(mouseOut).toBeCalled(); - expect(mouseOut).toHaveBeenCalledTimes(1); - expect(mouseOut.mock.calls[0][0]).toEqual(instance.getInner()); - document.body.removeChild(container); + expect(mouseOut).toBeCalled(); + expect(mouseOut).toHaveBeenCalledTimes(1); + expect(mouseOut.mock.calls[0][0]).toEqual(instance.getInner()); + } finally { + document.body.removeChild(container); + } }); // Regression test for https://github.com/facebook/react/pull/12877 @@ -320,42 +341,44 @@ describe('ReactDOMEventListener', () => { const container = document.createElement('div'); document.body.appendChild(container); - const imgRef = React.createRef(); - const videoRef = React.createRef(); + try { + const imgRef = React.createRef(); + const videoRef = React.createRef(); - const handleImgLoadStart = jest.fn(); - const handleVideoLoadStart = jest.fn(); - ReactDOM.render( -
- -
, - container, - ); + const handleImgLoadStart = jest.fn(); + const handleVideoLoadStart = jest.fn(); + ReactDOM.render( +
+ +
, + container, + ); - // Note for debugging: loadstart currently doesn't fire in Chrome. - // https://bugs.chromium.org/p/chromium/issues/detail?id=458851 - imgRef.current.dispatchEvent( - new ProgressEvent('loadstart', { - bubbles: false, - }), - ); - // Historically, we happened to not support onLoadStart - // on , 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 - // . So far nobody asked us for it. - expect(handleImgLoadStart).toHaveBeenCalledTimes(0); - - videoRef.current.dispatchEvent( - new ProgressEvent('loadstart', { - bubbles: false, - }), - ); - expect(handleVideoLoadStart).toHaveBeenCalledTimes(1); + // Note for debugging: loadstart currently doesn't fire in Chrome. + // https://bugs.chromium.org/p/chromium/issues/detail?id=458851 + imgRef.current.dispatchEvent( + new ProgressEvent('loadstart', { + bubbles: false, + }), + ); + // Historically, we happened to not support onLoadStart + // on , 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 + // . So far nobody asked us for it. + expect(handleImgLoadStart).toHaveBeenCalledTimes(0); - document.body.removeChild(container); + videoRef.current.dispatchEvent( + new ProgressEvent('loadstart', { + bubbles: false, + }), + ); + expect(handleVideoLoadStart).toHaveBeenCalledTimes(1); + } finally { + document.body.removeChild(container); + } }); it('should not attempt to listen to unnecessary events on the top level', () => {