From 507f0fb372705c4c6ac5288db3651019b1d0927b Mon Sep 17 00:00:00 2001 From: Sunil Pai Date: Wed, 21 Aug 2019 10:20:34 +0100 Subject: [PATCH 01/13] Revert "[ESLint] Forbid top-level use*() calls (#16455)" (#16522) This reverts commit 96eb703bbff49b7d52ad7d41ea18074dc8e7857a. --- .../__tests__/ESLintRulesOfHooks-test.js | 126 ++---------------- .../src/RulesOfHooks.js | 6 +- 2 files changed, 13 insertions(+), 119 deletions(-) diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js index c6fc1746e135..635196f8db52 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js @@ -19,17 +19,8 @@ ESLintTester.setDefaultConfig({ }, }); -// *************************************************** -// For easier local testing, you can add to any case: -// { -// skip: true, -// --or-- -// only: true, -// ... -// } -// *************************************************** - -const tests = { +const eslintTester = new ESLintTester(); +eslintTester.run('react-hooks', ReactHooksESLintRule, { valid: [ ` // Valid because components can use hooks. @@ -232,20 +223,21 @@ const tests = { (class {i() { useState(); }}); `, ` - // Valid because they're not matching use[A-Z]. - fooState(); + // Currently valid although we *could* consider these invalid. + // It doesn't make a lot of difference because it would crash early. use(); _use(); + useState(); _useState(); + use42(); + useHook(); use_hook(); + React.useState(); `, ` - // This is grey area. - // Currently it's valid (although React.useCallback would fail here). - // We could also get stricter and disallow it, just like we did - // with non-namespace use*() top-level calls. - const History = require('history-2.1.2'); - const browserHistory = History.useBasename(History.createHistory)({ + // Regression test for the popular "history" library + const {createHistory, useBasename} = require('history-2.1.2'); + const browserHistory = useBasename(createHistory)({ basename: '/', }); `, @@ -677,63 +669,8 @@ const tests = { conditionalError('useState'), ], }, - { - code: ` - // Invalid because it's dangerous and might not warn otherwise. - // This *must* be invalid. - function useHook({ bar }) { - let foo1 = bar && useState(); - let foo2 = bar || useState(); - let foo3 = bar ?? useState(); - } - `, - errors: [ - conditionalError('useState'), - conditionalError('useState'), - // TODO: ideally this *should* warn, but ESLint - // doesn't plan full support for ?? until it advances. - // conditionalError('useState'), - ], - }, - { - code: ` - // Invalid because it's dangerous. - // Normally, this would crash, but not if you use inline requires. - // This *must* be invalid. - // It's expected to have some false positives, but arguably - // they are confusing anyway due to the use*() convention - // already being associated with Hooks. - useState(); - if (foo) { - const foo = React.useCallback(() => {}); - } - useCustomHook(); - `, - errors: [ - topLevelError('useState'), - topLevelError('React.useCallback'), - topLevelError('useCustomHook'), - ], - }, - { - code: ` - // Technically this is a false positive. - // We *could* make it valid (and it used to be). - // - // However, top-level Hook-like calls can be very dangerous - // in environments with inline requires because they can mask - // the runtime error by accident. - // So we prefer to disallow it despite the false positive. - - const {createHistory, useBasename} = require('history-2.1.2'); - const browserHistory = useBasename(createHistory)({ - basename: '/', - }); - `, - errors: [topLevelError('useBasename')], - }, ], -}; +}); function conditionalError(hook, hasPreviousFinalizer = false) { return { @@ -771,42 +708,3 @@ function genericError(hook) { 'Hook function.', }; } - -function topLevelError(hook) { - return { - message: - `React Hook "${hook}" cannot be called at the top level. React Hooks ` + - 'must be called in a React function component or a custom React ' + - 'Hook function.', - }; -} - -// For easier local testing -if (!process.env.CI) { - let only = []; - let skipped = []; - [...tests.valid, ...tests.invalid].forEach(t => { - if (t.skip) { - delete t.skip; - skipped.push(t); - } - if (t.only) { - delete t.only; - only.push(t); - } - }); - const predicate = t => { - if (only.length > 0) { - return only.indexOf(t) !== -1; - } - if (skipped.length > 0) { - return skipped.indexOf(t) === -1; - } - return true; - }; - tests.valid = tests.valid.filter(predicate); - tests.invalid = tests.invalid.filter(predicate); -} - -const eslintTester = new ESLintTester(); -eslintTester.run('react-hooks', ReactHooksESLintRule, tests); diff --git a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js index fff4566cfbe6..405a6641a335 100644 --- a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js +++ b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js @@ -432,13 +432,9 @@ export default { 'React Hook function.'; context.report({node: hook, message}); } else if (codePathNode.type === 'Program') { + // For now, ignore if it's in top level scope. // We could warn here but there are false positives related // configuring libraries like `history`. - const message = - `React Hook "${context.getSource(hook)}" cannot be called ` + - 'at the top level. React Hooks must be called in a ' + - 'React function component or a custom React Hook function.'; - context.report({node: hook, message}); } else { // Assume in all other cases the user called a hook in some // random function callback. This should usually be true for From c433fbb593c7f42263c0209d3ff421aea59fca4c Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 21 Aug 2019 15:43:31 +0100 Subject: [PATCH 02/13] Revert "Revert "[ESLint] Forbid top-level use*() calls (#16455)"" (#16525) * Revert "Revert "[ESLint] Forbid top-level use*() calls (#16455)" (#16522)" This reverts commit 507f0fb372705c4c6ac5288db3651019b1d0927b. * Update RulesOfHooks.js --- .../__tests__/ESLintRulesOfHooks-test.js | 126 ++++++++++++++++-- .../src/RulesOfHooks.js | 9 +- 2 files changed, 120 insertions(+), 15 deletions(-) diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js index 635196f8db52..c6fc1746e135 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js @@ -19,8 +19,17 @@ ESLintTester.setDefaultConfig({ }, }); -const eslintTester = new ESLintTester(); -eslintTester.run('react-hooks', ReactHooksESLintRule, { +// *************************************************** +// For easier local testing, you can add to any case: +// { +// skip: true, +// --or-- +// only: true, +// ... +// } +// *************************************************** + +const tests = { valid: [ ` // Valid because components can use hooks. @@ -223,21 +232,20 @@ eslintTester.run('react-hooks', ReactHooksESLintRule, { (class {i() { useState(); }}); `, ` - // Currently valid although we *could* consider these invalid. - // It doesn't make a lot of difference because it would crash early. + // Valid because they're not matching use[A-Z]. + fooState(); use(); _use(); - useState(); _useState(); - use42(); - useHook(); use_hook(); - React.useState(); `, ` - // Regression test for the popular "history" library - const {createHistory, useBasename} = require('history-2.1.2'); - const browserHistory = useBasename(createHistory)({ + // This is grey area. + // Currently it's valid (although React.useCallback would fail here). + // We could also get stricter and disallow it, just like we did + // with non-namespace use*() top-level calls. + const History = require('history-2.1.2'); + const browserHistory = History.useBasename(History.createHistory)({ basename: '/', }); `, @@ -669,8 +677,63 @@ eslintTester.run('react-hooks', ReactHooksESLintRule, { conditionalError('useState'), ], }, + { + code: ` + // Invalid because it's dangerous and might not warn otherwise. + // This *must* be invalid. + function useHook({ bar }) { + let foo1 = bar && useState(); + let foo2 = bar || useState(); + let foo3 = bar ?? useState(); + } + `, + errors: [ + conditionalError('useState'), + conditionalError('useState'), + // TODO: ideally this *should* warn, but ESLint + // doesn't plan full support for ?? until it advances. + // conditionalError('useState'), + ], + }, + { + code: ` + // Invalid because it's dangerous. + // Normally, this would crash, but not if you use inline requires. + // This *must* be invalid. + // It's expected to have some false positives, but arguably + // they are confusing anyway due to the use*() convention + // already being associated with Hooks. + useState(); + if (foo) { + const foo = React.useCallback(() => {}); + } + useCustomHook(); + `, + errors: [ + topLevelError('useState'), + topLevelError('React.useCallback'), + topLevelError('useCustomHook'), + ], + }, + { + code: ` + // Technically this is a false positive. + // We *could* make it valid (and it used to be). + // + // However, top-level Hook-like calls can be very dangerous + // in environments with inline requires because they can mask + // the runtime error by accident. + // So we prefer to disallow it despite the false positive. + + const {createHistory, useBasename} = require('history-2.1.2'); + const browserHistory = useBasename(createHistory)({ + basename: '/', + }); + `, + errors: [topLevelError('useBasename')], + }, ], -}); +}; function conditionalError(hook, hasPreviousFinalizer = false) { return { @@ -708,3 +771,42 @@ function genericError(hook) { 'Hook function.', }; } + +function topLevelError(hook) { + return { + message: + `React Hook "${hook}" cannot be called at the top level. React Hooks ` + + 'must be called in a React function component or a custom React ' + + 'Hook function.', + }; +} + +// For easier local testing +if (!process.env.CI) { + let only = []; + let skipped = []; + [...tests.valid, ...tests.invalid].forEach(t => { + if (t.skip) { + delete t.skip; + skipped.push(t); + } + if (t.only) { + delete t.only; + only.push(t); + } + }); + const predicate = t => { + if (only.length > 0) { + return only.indexOf(t) !== -1; + } + if (skipped.length > 0) { + return skipped.indexOf(t) === -1; + } + return true; + }; + tests.valid = tests.valid.filter(predicate); + tests.invalid = tests.invalid.filter(predicate); +} + +const eslintTester = new ESLintTester(); +eslintTester.run('react-hooks', ReactHooksESLintRule, tests); diff --git a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js index 405a6641a335..a4a4063a8b01 100644 --- a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js +++ b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js @@ -432,9 +432,12 @@ export default { 'React Hook function.'; context.report({node: hook, message}); } else if (codePathNode.type === 'Program') { - // For now, ignore if it's in top level scope. - // We could warn here but there are false positives related - // configuring libraries like `history`. + // These are dangerous if you have inline requires enabled. + const message = + `React Hook "${context.getSource(hook)}" cannot be called ` + + 'at the top level. React Hooks must be called in a ' + + 'React function component or a custom React Hook function.'; + context.report({node: hook, message}); } else { // Assume in all other cases the user called a hook in some // random function callback. This should usually be true for From 2559111c21a4bb18ab6ade5e464cb42863e9e8b3 Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Wed, 21 Aug 2019 10:07:15 -0700 Subject: [PATCH 03/13] [react-events] Rely on 'buttons' rather than 'button' (#16479) The semantics of 'button' on events differs between PointerEvent and MouseEvent, whereas they are the same for 'buttons'. Furthermore, 'buttons' allows developers to determine when multiple buttons are pressed as the same time. https://w3c.github.io/pointerevents/#the-button-property --- .../src/events/DOMEventResponderSystem.js | 4 +- packages/react-events/docs/Press.md | 10 --- packages/react-events/src/dom/ContextMenu.js | 31 +++++---- packages/react-events/src/dom/Hover.js | 8 +-- packages/react-events/src/dom/Press.js | 66 ++++++++++--------- .../__tests__/ContextMenu-test.internal.js | 25 +++++-- .../src/dom/__tests__/Press-test.internal.js | 49 ++++++++------ .../src/dom/testing-library/domEnvironment.js | 24 +++++++ .../dom/testing-library/domEventSequences.js | 31 +++++---- .../src/dom/testing-library/domEvents.js | 65 ++++++++++++------ .../src/dom/testing-library/index.js | 15 ++++- 11 files changed, 205 insertions(+), 123 deletions(-) diff --git a/packages/react-dom/src/events/DOMEventResponderSystem.js b/packages/react-dom/src/events/DOMEventResponderSystem.js index cc82d658b6e3..e6db323c6349 100644 --- a/packages/react-dom/src/events/DOMEventResponderSystem.js +++ b/packages/react-dom/src/events/DOMEventResponderSystem.js @@ -445,7 +445,7 @@ function createDOMResponderEvent( passive: boolean, passiveSupported: boolean, ): ReactDOMResponderEvent { - const {pointerType} = (nativeEvent: any); + const {buttons, pointerType} = (nativeEvent: any); let eventPointerType = ''; let pointerId = null; @@ -454,7 +454,7 @@ function createDOMResponderEvent( pointerId = (nativeEvent: any).pointerId; } else if (nativeEvent.key !== undefined) { eventPointerType = 'keyboard'; - } else if (nativeEvent.button !== undefined) { + } else if (buttons !== undefined) { eventPointerType = 'mouse'; } else if ((nativeEvent: any).changedTouches !== undefined) { eventPointerType = 'touch'; diff --git a/packages/react-events/docs/Press.md b/packages/react-events/docs/Press.md index 50e6c2b94501..733a932a7a75 100644 --- a/packages/react-events/docs/Press.md +++ b/packages/react-events/docs/Press.md @@ -78,11 +78,6 @@ type PressOffset = { Disables all `Press` events. -### onContextMenu: (e: PressEvent) => void - -Called when the context menu is shown. When a press is active, the context menu -will only be shown (and the press cancelled) if `preventDefault` is `false`. - ### onPress: (e: PressEvent) => void Called immediately after a press is released, unless the press is released @@ -115,11 +110,6 @@ down) can be moved back within the bounds of the element to reactivate it. Ensure you pass in a constant to reduce memory allocations. Default is `20` for each offset. -### preventContextMenu: boolean = false - -Prevents the native context menu from being shown, but `onContextMenu` -is still called. - ### preventDefault: boolean = true Whether to `preventDefault()` native events. Native behavior is prevented by diff --git a/packages/react-events/src/dom/ContextMenu.js b/packages/react-events/src/dom/ContextMenu.js index b8094b71d8b4..40cd079e89d7 100644 --- a/packages/react-events/src/dom/ContextMenu.js +++ b/packages/react-events/src/dom/ContextMenu.js @@ -28,20 +28,19 @@ type ContextMenuState = { }; type ContextMenuEvent = {| - target: Element | Document, - type: 'contextmenu', - pointerType: PointerType, - timeStamp: number, - clientX: null | number, - clientY: null | number, - pageX: null | number, - pageY: null | number, - x: null | number, - y: null | number, altKey: boolean, + buttons: 0 | 1 | 2, ctrlKey: boolean, metaKey: boolean, + pageX: null | number, + pageY: null | number, + pointerType: PointerType, shiftKey: boolean, + target: Element | Document, + timeStamp: number, + type: 'contextmenu', + x: null | number, + y: null | number, |}; const hasPointerEvents = @@ -60,13 +59,13 @@ function dispatchContextMenuEvent( const gestureState = { altKey: nativeEvent.altKey, - button: nativeEvent.button === 0 ? 'primary' : 'auxillary', - ctrlKey: nativeEvent.altKey, - metaKey: nativeEvent.altKey, - pageX: nativeEvent.altKey, - pageY: nativeEvent.altKey, + buttons: nativeEvent.buttons != null ? nativeEvent.buttons : 0, + ctrlKey: nativeEvent.ctrlKey, + metaKey: nativeEvent.metaKey, + pageX: nativeEvent.pageX, + pageY: nativeEvent.pageY, pointerType, - shiftKey: nativeEvent.altKey, + shiftKey: nativeEvent.shiftKey, target, timeStamp, type: 'contextmenu', diff --git a/packages/react-events/src/dom/Hover.js b/packages/react-events/src/dom/Hover.js index 6e47d33e4ab2..dea896042b92 100644 --- a/packages/react-events/src/dom/Hover.js +++ b/packages/react-events/src/dom/Hover.js @@ -37,16 +37,16 @@ type HoverState = { type HoverEventType = 'hoverstart' | 'hoverend' | 'hoverchange' | 'hovermove'; type HoverEvent = {| - pointerType: PointerType, - target: Element | Document, - type: HoverEventType, - timeStamp: number, clientX: null | number, clientY: null | number, pageX: null | number, pageY: null | number, + pointerType: PointerType, screenX: null | number, screenY: null | number, + target: Element | Document, + timeStamp: number, + type: HoverEventType, x: null | number, y: null | number, |}; diff --git a/packages/react-events/src/dom/Press.js b/packages/react-events/src/dom/Press.js index c14c73935e43..3a08adc280c2 100644 --- a/packages/react-events/src/dom/Press.js +++ b/packages/react-events/src/dom/Press.js @@ -75,24 +75,24 @@ type PressEventType = | 'presschange'; type PressEvent = {| - button: 'primary' | 'auxillary', - defaultPrevented: boolean, - target: Element | Document, - type: PressEventType, - pointerType: PointerType, - timeStamp: number, + altKey: boolean, + buttons: 0 | 1 | 4, clientX: null | number, clientY: null | number, + ctrlKey: boolean, + defaultPrevented: boolean, + metaKey: boolean, pageX: null | number, pageY: null | number, + pointerType: PointerType, screenX: null | number, screenY: null | number, + shiftKey: boolean, + target: Element | Document, + timeStamp: number, + type: PressEventType, x: null | number, y: null | number, - altKey: boolean, - ctrlKey: boolean, - metaKey: boolean, - shiftKey: boolean, |}; const hasPointerEvents = @@ -151,7 +151,7 @@ function createPressEvent( defaultPrevented: boolean, ): PressEvent { const timeStamp = context.getTimeStamp(); - let button = 'primary'; + let buttons = 1; let clientX = null; let clientY = null; let pageX = null; @@ -171,31 +171,36 @@ function createPressEvent( let eventObject; eventObject = (touchEvent: any) || (nativeEvent: any); if (eventObject) { - ({clientX, clientY, pageX, pageY, screenX, screenY} = eventObject); - } - if (nativeEvent.button === 1) { - button = 'auxillary'; + ({ + buttons, + clientX, + clientY, + pageX, + pageY, + screenX, + screenY, + } = eventObject); } } return { - button, - defaultPrevented, - target, - type, - pointerType, - timeStamp, + altKey, + buttons, clientX, clientY, + ctrlKey, + defaultPrevented, + metaKey, pageX, pageY, + pointerType, screenX, screenY, + shiftKey, + target, + timeStamp, + type, x: clientX, y: clientY, - altKey, - ctrlKey, - metaKey, - shiftKey, }; } @@ -581,11 +586,12 @@ const pressResponderImpl = { state.activePointerId = touchEvent.identifier; } - // Ignore any device buttons except primary/auxillary and touch/pen contact. + // Ignore any device buttons except primary/secondary and touch/pen contact. // Additionally we ignore primary-button + ctrl-key with Macs as that // acts like right-click and opens the contextmenu. if ( - nativeEvent.button > 1 || + nativeEvent.buttons === 2 || + nativeEvent.buttons > 4 || (isMac && isMouseEvent && nativeEvent.ctrlKey) ) { return; @@ -703,7 +709,7 @@ const pressResponderImpl = { case 'mouseup': case 'touchend': { if (isPressed) { - const button = nativeEvent.button; + const buttons = nativeEvent.buttons; let isKeyboardEvent = false; let touchEvent; if (type === 'pointerup' && activePointerId !== pointerId) { @@ -722,7 +728,7 @@ const pressResponderImpl = { } isKeyboardEvent = true; removeRootEventTypes(context, state); - } else if (button === 1) { + } else if (buttons === 4) { // Remove the root events here as no 'click' event is dispatched when this 'button' is pressed. removeRootEventTypes(context, state); } @@ -780,7 +786,7 @@ const pressResponderImpl = { } } - if (state.isPressWithinResponderRegion && button !== 1) { + if (state.isPressWithinResponderRegion && buttons !== 4) { dispatchEvent( event, onPress, diff --git a/packages/react-events/src/dom/__tests__/ContextMenu-test.internal.js b/packages/react-events/src/dom/__tests__/ContextMenu-test.internal.js index b25cbf29fcbd..d64b78ade76c 100644 --- a/packages/react-events/src/dom/__tests__/ContextMenu-test.internal.js +++ b/packages/react-events/src/dom/__tests__/ContextMenu-test.internal.js @@ -9,7 +9,12 @@ 'use strict'; -import {createEventTarget, platform, setPointerEvent} from '../testing-library'; +import { + buttonsType, + createEventTarget, + platform, + setPointerEvent, +} from '../testing-library'; let React; let ReactFeatureFlags; @@ -61,7 +66,11 @@ describe.each(table)('ContextMenu responder', hasPointerEvents => { expect(preventDefault).toHaveBeenCalledTimes(1); expect(onContextMenu).toHaveBeenCalledTimes(1); expect(onContextMenu).toHaveBeenCalledWith( - expect.objectContaining({pointerType: 'mouse', type: 'contextmenu'}), + expect.objectContaining({ + buttons: buttonsType.secondary, + pointerType: 'mouse', + type: 'contextmenu', + }), ); }); @@ -80,7 +89,11 @@ describe.each(table)('ContextMenu responder', hasPointerEvents => { expect(preventDefault).toHaveBeenCalledTimes(1); expect(onContextMenu).toHaveBeenCalledTimes(1); expect(onContextMenu).toHaveBeenCalledWith( - expect.objectContaining({pointerType: 'touch', type: 'contextmenu'}), + expect.objectContaining({ + buttons: buttonsType.none, + pointerType: 'touch', + type: 'contextmenu', + }), ); }); @@ -144,7 +157,11 @@ describe.each(table)('ContextMenu responder', hasPointerEvents => { target.contextmenu({}, {modified: true}); expect(onContextMenu).toHaveBeenCalledTimes(1); expect(onContextMenu).toHaveBeenCalledWith( - expect.objectContaining({pointerType: 'mouse', type: 'contextmenu'}), + expect.objectContaining({ + buttons: buttonsType.primary, + pointerType: 'mouse', + type: 'contextmenu', + }), ); }); }); diff --git a/packages/react-events/src/dom/__tests__/Press-test.internal.js b/packages/react-events/src/dom/__tests__/Press-test.internal.js index 10ddd2af356c..34e483e5a54c 100644 --- a/packages/react-events/src/dom/__tests__/Press-test.internal.js +++ b/packages/react-events/src/dom/__tests__/Press-test.internal.js @@ -9,7 +9,11 @@ 'use strict'; -import {createEventTarget, setPointerEvent} from '../testing-library'; +import { + buttonsType, + createEventTarget, + setPointerEvent, +} from '../testing-library'; let React; let ReactFeatureFlags; @@ -114,36 +118,36 @@ describe.each(environmentTable)('Press responder', hasPointerEvents => { }, ); - it('is called after auxillary-button pointer down', () => { + it('is called after middle-button pointer down', () => { const target = createEventTarget(ref.current); - target.pointerdown({button: 1, pointerType: 'mouse'}); + target.pointerdown({buttons: buttonsType.middle, pointerType: 'mouse'}); expect(onPressStart).toHaveBeenCalledTimes(1); expect(onPressStart).toHaveBeenCalledWith( expect.objectContaining({ - button: 'auxillary', + buttons: buttonsType.middle, pointerType: 'mouse', type: 'pressstart', }), ); }); - it('is not called after pointer move following auxillary-button press', () => { + it('is not called after pointer move following middle-button press', () => { const node = ref.current; const target = createEventTarget(node); target.setBoundingClientRect({x: 0, y: 0, width: 100, height: 100}); - target.pointerdown({button: 1, pointerType: 'mouse'}); - target.pointerup({button: 1, pointerType: 'mouse'}); + target.pointerdown({buttons: buttonsType.middle, pointerType: 'mouse'}); + target.pointerup({buttons: buttonsType.middle, pointerType: 'mouse'}); target.pointerhover({x: 110, y: 110}); target.pointerhover({x: 50, y: 50}); expect(onPressStart).toHaveBeenCalledTimes(1); }); - it('ignores any events not caused by primary/auxillary-click or touch/pen contact', () => { + it('ignores any events not caused by primary/middle-click or touch/pen contact', () => { const target = createEventTarget(ref.current); - target.pointerdown({button: 2}); - target.pointerup({button: 2}); - target.pointerdown({button: 5}); - target.pointerup({button: 5}); + target.pointerdown({buttons: buttonsType.secondary}); + target.pointerup({buttons: buttonsType.secondary}); + target.pointerdown({buttons: buttonsType.eraser}); + target.pointerup({buttons: buttonsType.eraser}); expect(onPressStart).toHaveBeenCalledTimes(0); }); @@ -209,14 +213,14 @@ describe.each(environmentTable)('Press responder', hasPointerEvents => { }, ); - it('is called after auxillary-button pointer up', () => { + it('is called after middle-button pointer up', () => { const target = createEventTarget(ref.current); - target.pointerdown({button: 1, pointerType: 'mouse'}); - target.pointerup({button: 1, pointerType: 'mouse'}); + target.pointerdown({buttons: buttonsType.middle, pointerType: 'mouse'}); + target.pointerup({buttons: buttonsType.middle, pointerType: 'mouse'}); expect(onPressEnd).toHaveBeenCalledTimes(1); expect(onPressEnd).toHaveBeenCalledWith( expect.objectContaining({ - button: 'auxillary', + buttons: buttonsType.middle, pointerType: 'mouse', type: 'pressend', }), @@ -350,10 +354,10 @@ describe.each(environmentTable)('Press responder', hasPointerEvents => { }, ); - it('is not called after auxillary-button press', () => { + it('is not called after middle-button press', () => { const target = createEventTarget(ref.current); - target.pointerdown({button: 1, pointerType: 'mouse'}); - target.pointerup({button: 1, pointerType: 'mouse'}); + target.pointerdown({buttons: buttonsType.middle, pointerType: 'mouse'}); + target.pointerup({buttons: buttonsType.middle, pointerType: 'mouse'}); expect(onPress).not.toHaveBeenCalled(); }); @@ -460,7 +464,12 @@ describe.each(environmentTable)('Press responder', hasPointerEvents => { const target = createEventTarget(ref.current); target.setBoundingClientRect({x: 0, y: 0, width: 100, height: 100}); target.keydown({key: 'Enter'}); - target.pointermove({button: -1, pointerType: 'mouse', x: 10, y: 10}); + target.pointermove({ + buttons: buttonsType.none, + pointerType: 'mouse', + x: 10, + y: 10, + }); expect(onPressMove).not.toBeCalled(); }); }); diff --git a/packages/react-events/src/dom/testing-library/domEnvironment.js b/packages/react-events/src/dom/testing-library/domEnvironment.js index 035673ae81ac..f0a65576a94c 100644 --- a/packages/react-events/src/dom/testing-library/domEnvironment.js +++ b/packages/react-events/src/dom/testing-library/domEnvironment.js @@ -8,6 +8,7 @@ */ 'use strict'; + /** * Change environment support for PointerEvent. */ @@ -49,3 +50,26 @@ export const platform = { } }, }; + +/** + * Buttons bitmask + */ + +export const buttonsType = { + none: 0, + // left-mouse + // touch contact + // pen contact + primary: 1, + // right-mouse + // pen barrel button + secondary: 2, + // middle mouse + middle: 4, + // back mouse + back: 8, + // forward mouse + forward: 16, + // pen eraser + eraser: 32, +}; diff --git a/packages/react-events/src/dom/testing-library/domEventSequences.js b/packages/react-events/src/dom/testing-library/domEventSequences.js index d84feacc5ebf..ebda2ec05779 100644 --- a/packages/react-events/src/dom/testing-library/domEventSequences.js +++ b/packages/react-events/src/dom/testing-library/domEventSequences.js @@ -10,7 +10,7 @@ 'use strict'; import * as domEvents from './domEvents'; -import {hasPointerEvent, platform} from './domEnvironment'; +import {buttonsType, hasPointerEvent, platform} from './domEnvironment'; function emptyFunction() {} @@ -29,30 +29,33 @@ export function contextmenu( ) { const dispatch = arg => target.dispatchEvent(arg); if (pointerType === 'touch') { - const button = 0; if (hasPointerEvent()) { - dispatch(domEvents.pointerdown({button, pointerType})); + dispatch( + domEvents.pointerdown({buttons: buttonsType.primary, pointerType}), + ); } dispatch(domEvents.touchstart()); - dispatch(domEvents.contextmenu({button, preventDefault})); + dispatch( + domEvents.contextmenu({buttons: buttonsType.none, preventDefault}), + ); } else if (pointerType === 'mouse') { if (modified === true) { - const button = 0; + const buttons = buttonsType.primary; const ctrlKey = true; if (hasPointerEvent()) { - dispatch(domEvents.pointerdown({button, ctrlKey, pointerType})); + dispatch(domEvents.pointerdown({buttons, ctrlKey, pointerType})); } - dispatch(domEvents.mousedown({button, ctrlKey})); + dispatch(domEvents.mousedown({buttons, ctrlKey})); if (platform.get() === 'mac') { - dispatch(domEvents.contextmenu({button, ctrlKey, preventDefault})); + dispatch(domEvents.contextmenu({buttons, ctrlKey, preventDefault})); } } else { - const button = 2; + const buttons = buttonsType.secondary; if (hasPointerEvent()) { - dispatch(domEvents.pointerdown({button, pointerType})); + dispatch(domEvents.pointerdown({buttons, pointerType})); } - dispatch(domEvents.mousedown({button})); - dispatch(domEvents.contextmenu({button, preventDefault})); + dispatch(domEvents.mousedown({buttons})); + dispatch(domEvents.contextmenu({buttons, preventDefault})); } } } @@ -74,7 +77,7 @@ export function pointercancel(target, payload) { export function pointerdown(target, defaultPayload) { const dispatch = arg => target.dispatchEvent(arg); const pointerType = getPointerType(defaultPayload); - const payload = {button: 0, buttons: 2, ...defaultPayload}; + const payload = {buttons: buttonsType.primary, ...defaultPayload}; if (pointerType === 'mouse') { if (hasPointerEvent()) { @@ -147,7 +150,7 @@ export function pointermove(target, payload) { export function pointerup(target, defaultPayload) { const dispatch = arg => target.dispatchEvent(arg); const pointerType = getPointerType(defaultPayload); - const payload = {button: 0, buttons: 2, ...defaultPayload}; + const payload = {buttons: buttonsType.none, ...defaultPayload}; if (pointerType === 'mouse') { if (hasPointerEvent()) { diff --git a/packages/react-events/src/dom/testing-library/domEvents.js b/packages/react-events/src/dom/testing-library/domEvents.js index 01ed26a56890..288884531f4f 100644 --- a/packages/react-events/src/dom/testing-library/domEvents.js +++ b/packages/react-events/src/dom/testing-library/domEvents.js @@ -9,6 +9,8 @@ 'use strict'; +import {buttonsType} from './domEnvironment'; + /** * Native event object mocks for higher-level events. * @@ -24,6 +26,9 @@ * 3. PointerEvent and TouchEvent fields are normalized (e.g., 'rotationAngle' -> 'twist') */ +const defaultPointerSize = 23; +const defaultBrowserChromeSize = 50; + function emptyFunction() {} function createEvent(type, data = {}) { @@ -57,8 +62,7 @@ function createPointerEvent( type, { altKey = false, - button = -1, - buttons = 0, + buttons = buttonsType.none, ctrlKey = false, height, metaKey = false, @@ -86,7 +90,6 @@ function createPointerEvent( return createEvent(type, { altKey, - button, buttons, clientX: x, clientY: y, @@ -94,7 +97,12 @@ function createPointerEvent( getModifierState(keyArg) { createGetModifierState(keyArg, modifierState); }, - height: pointerType === 'mouse' ? 1 : height != null ? height : 11.5, + height: + pointerType === 'mouse' + ? 1 + : height != null + ? height + : defaultPointerSize, metaKey, movementX, movementY, @@ -107,13 +115,14 @@ function createPointerEvent( pressure, preventDefault, screenX: x, - screenY: y + 50, // arbitrary value to emulate browser chrome, etc + screenY: y + defaultBrowserChromeSize, shiftKey, tangentialPressure, tiltX, tiltY, twist, - width: pointerType === 'mouse' ? 1 : width != null ? width : 11.5, + width: + pointerType === 'mouse' ? 1 : width != null ? width : defaultPointerSize, }); } @@ -147,8 +156,7 @@ function createMouseEvent( type, { altKey = false, - button = -1, - buttons = 0, + buttons = buttonsType.none, ctrlKey = false, metaKey = false, movementX = 0, @@ -167,7 +175,6 @@ function createMouseEvent( return createEvent(type, { altKey, - button, buttons, clientX: x, clientY: y, @@ -184,7 +191,7 @@ function createMouseEvent( pageY: pageY || y, preventDefault, screenX: x, - screenY: y + 50, // arbitrary value to emulate browser chrome, etc + screenY: y + defaultBrowserChromeSize, shiftKey, }); } @@ -194,7 +201,7 @@ function createTouchEvent( { altKey = false, ctrlKey = false, - height = 11.5, + height = defaultPointerSize, metaKey = false, pageX, pageY, @@ -202,7 +209,7 @@ function createTouchEvent( preventDefault = emptyFunction, shiftKey = false, twist = 0, - width = 11.5, + width = defaultPointerSize, x = 0, y = 0, } = {}, @@ -214,13 +221,15 @@ function createTouchEvent( identifier: pointerId, pageX: pageX || x, pageY: pageY || y, - radiusX: width, - radiusY: height, + radiusX: width / 2, + radiusY: height / 2, rotationAngle: twist, screenX: x, - screenY: y + 50, // arbitrary value to emulate browser chrome, etc + screenY: y + defaultBrowserChromeSize, }; + const activeTouch = type !== 'touchend' ? [touch] : null; + return createEvent(type, { altKey, changedTouches: [touch], @@ -228,8 +237,8 @@ function createTouchEvent( metaKey, preventDefault, shiftKey, - targetTouches: type !== 'touchend' ? [touch] : null, - touches: [touch], + targetTouches: activeTouch, + touches: activeTouch, }); } @@ -290,7 +299,12 @@ export function pointercancel(payload) { } export function pointerdown(payload) { - return createPointerEvent('pointerdown', {button: 0, buttons: 2, ...payload}); + const isTouch = payload != null && payload.pointerType === 'mouse'; + return createPointerEvent('pointerdown', { + buttons: buttonsType.primary, + pressure: isTouch ? 1 : 0.5, + ...payload, + }); } export function pointerenter(payload) { @@ -314,7 +328,10 @@ export function pointerover(payload) { } export function pointerup(payload) { - return createPointerEvent('pointerup', {button: 0, buttons: 2, ...payload}); + return createPointerEvent('pointerup', { + buttons: buttonsType.none, + ...payload, + }); } /** @@ -322,7 +339,10 @@ export function pointerup(payload) { */ export function mousedown(payload) { - return createMouseEvent('mousedown', {button: 0, buttons: 2, ...payload}); + return createMouseEvent('mousedown', { + buttons: buttonsType.primary, + ...payload, + }); } export function mouseenter(payload) { @@ -346,7 +366,10 @@ export function mouseover(payload) { } export function mouseup(payload) { - return createMouseEvent('mouseup', {button: 0, buttons: 2, ...payload}); + return createMouseEvent('mouseup', { + buttons: buttonsType.none, + ...payload, + }); } /** diff --git a/packages/react-events/src/dom/testing-library/index.js b/packages/react-events/src/dom/testing-library/index.js index cb549dc5b53c..e1377f6d70e0 100644 --- a/packages/react-events/src/dom/testing-library/index.js +++ b/packages/react-events/src/dom/testing-library/index.js @@ -11,7 +11,12 @@ import * as domEvents from './domEvents'; import * as domEventSequences from './domEventSequences'; -import {hasPointerEvent, setPointerEvent, platform} from './domEnvironment'; +import { + buttonsType, + hasPointerEvent, + setPointerEvent, + platform, +} from './domEnvironment'; const createEventTarget = node => ({ node, @@ -101,4 +106,10 @@ const createEventTarget = node => ({ }, }); -export {createEventTarget, platform, hasPointerEvent, setPointerEvent}; +export { + buttonsType, + createEventTarget, + platform, + hasPointerEvent, + setPointerEvent, +}; From 06728290533bce24b0893461c3f4ddc1547f85b4 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 21 Aug 2019 19:14:34 +0100 Subject: [PATCH 04/13] Bump ESLint plugin to 2.0 (#16528) --- CHANGELOG.md | 4 ++++ packages/eslint-plugin-react-hooks/package.json | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 80180a8e4e3d..cf391ed9f135 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,10 @@ * Warn in Strict Mode if effects are scheduled outside an `act()` call. ([@threepointone](https://github.com/threepointone) in [#15763](https://github.com/facebook/react/pull/15763) and [#16041](https://github.com/facebook/react/pull/16041)) * Warn when using `act` from the wrong renderer. ([@threepointone](https://github.com/threepointone) in [#15756](https://github.com/facebook/react/pull/15756)) +### ESLint Plugin: React Hooks + +* Report Hook calls at the top level as a violation. ([gaearon](https://github.com/gaearon) in [#16455](https://github.com/facebook/react/pull/16455)) + ## 16.8.6 (March 27, 2019) ### React DOM diff --git a/packages/eslint-plugin-react-hooks/package.json b/packages/eslint-plugin-react-hooks/package.json index dd970825f918..603b6c8574ef 100644 --- a/packages/eslint-plugin-react-hooks/package.json +++ b/packages/eslint-plugin-react-hooks/package.json @@ -1,7 +1,7 @@ { "name": "eslint-plugin-react-hooks", "description": "ESLint rules for React Hooks", - "version": "1.7.0", + "version": "2.0.0", "repository": { "type": "git", "url": "https://github.com/facebook/react.git", From 3ed289b3b1286007e3f594b0b767b3b64bf3f86c Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 21 Aug 2019 19:49:22 +0100 Subject: [PATCH 05/13] Clear canceled task node early (#16403) --- .../react-reconciler/src/SchedulerWithReactIntegration.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/SchedulerWithReactIntegration.js b/packages/react-reconciler/src/SchedulerWithReactIntegration.js index fc948f3f908a..3447fdcaebf2 100644 --- a/packages/react-reconciler/src/SchedulerWithReactIntegration.js +++ b/packages/react-reconciler/src/SchedulerWithReactIntegration.js @@ -160,7 +160,9 @@ export function cancelCallback(callbackNode: mixed) { export function flushSyncCallbackQueue() { if (immediateQueueCallbackNode !== null) { - Scheduler_cancelCallback(immediateQueueCallbackNode); + const node = immediateQueueCallbackNode; + immediateQueueCallbackNode = null; + Scheduler_cancelCallback(node); } flushSyncCallbackQueueImpl(); } From 8a01b50fc316761ebc3250d7b3c46c01482ef06c Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 21 Aug 2019 20:48:15 +0100 Subject: [PATCH 06/13] eslint-plugin-react-hooks@2.0.1 --- packages/eslint-plugin-react-hooks/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin-react-hooks/package.json b/packages/eslint-plugin-react-hooks/package.json index 603b6c8574ef..293a9c44ee1a 100644 --- a/packages/eslint-plugin-react-hooks/package.json +++ b/packages/eslint-plugin-react-hooks/package.json @@ -1,7 +1,7 @@ { "name": "eslint-plugin-react-hooks", "description": "ESLint rules for React Hooks", - "version": "2.0.0", + "version": "2.0.1", "repository": { "type": "git", "url": "https://github.com/facebook/react.git", From 05f5192e8106d006cc3189ae68c523ca123ae297 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 22 Aug 2019 08:46:20 -0700 Subject: [PATCH 07/13] [Partial Hydration] Dispatching events should not work until hydration commits (#16532) * Refactor a bit to use less property access * Add test for invoking an event before mount * Add Hydration effect tag This is equivalent to a "Placement" effect in that it's a new insertion to the tree but it doesn't need an actual mutation. It is only used to determine if a subtree has actually mounted yet. * Use the Hydration flag for Roots Previous roots had a Placement flag on them as a hack for this case but since we have a special flag for it now, we can just use that. * Add Flare test --- ...DOMServerPartialHydration-test.internal.js | 166 ++++++++++++++++++ .../ReactServerRenderingHydration-test.js | 87 +++++++++ .../src/client/ReactDOMComponentTree.js | 25 +-- .../src/ReactFiberBeginWork.js | 24 ++- .../src/ReactFiberCompleteWork.js | 17 +- .../src/ReactFiberTreeReflection.js | 19 +- .../src/ReactFiberWorkLoop.js | 17 +- packages/shared/ReactSideEffectTags.js | 34 ++-- 8 files changed, 335 insertions(+), 54 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index 11c6ad904b9b..d31bf6cfc9a4 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -25,6 +25,7 @@ describe('ReactDOMServerPartialHydration', () => { ReactFeatureFlags = require('shared/ReactFeatureFlags'); ReactFeatureFlags.enableSuspenseServerRenderer = true; ReactFeatureFlags.enableSuspenseCallback = true; + ReactFeatureFlags.enableFlareAPI = true; React = require('react'); ReactDOM = require('react-dom'); @@ -1729,4 +1730,169 @@ describe('ReactDOMServerPartialHydration', () => { // patched up the tree, which might mean we haven't patched the className. expect(newSpan.className).toBe('hi'); }); + + it('does not invoke an event on a hydrated node until it commits', async () => { + let suspend = false; + let resolve; + let promise = new Promise(resolvePromise => (resolve = resolvePromise)); + + function Sibling({text}) { + if (suspend) { + throw promise; + } else { + return 'Hello'; + } + } + + let clicks = 0; + + function Button() { + let [clicked, setClicked] = React.useState(false); + if (clicked) { + return null; + } + return ( + { + setClicked(true); + clicks++; + }}> + Click me + + ); + } + + function App() { + return ( +
+ +
+ ); + } + + suspend = false; + let finalHTML = ReactDOMServer.renderToString(); + let container = document.createElement('div'); + container.innerHTML = finalHTML; + + // We need this to be in the document since we'll dispatch events on it. + document.body.appendChild(container); + + let a = container.getElementsByTagName('a')[0]; + + // On the client we don't have all data yet but we want to start + // hydrating anyway. + suspend = true; + let root = ReactDOM.unstable_createRoot(container, {hydrate: true}); + root.render(); + Scheduler.unstable_flushAll(); + jest.runAllTimers(); + + expect(container.textContent).toBe('Click meHello'); + + // We're now partially hydrated. + a.click(); + expect(clicks).toBe(0); + + // Resolving the promise so that rendering can complete. + suspend = false; + resolve(); + await promise; + + Scheduler.unstable_flushAll(); + jest.runAllTimers(); + + // TODO: With selective hydration the event should've been replayed + // but for now we'll have to issue it again. + act(() => { + a.click(); + }); + + expect(clicks).toBe(1); + + expect(container.textContent).toBe('Hello'); + + document.body.removeChild(container); + }); + + it('does not invoke an event on a hydrated EventResponder until it commits', async () => { + let suspend = false; + let resolve; + let promise = new Promise(resolvePromise => (resolve = resolvePromise)); + + function Sibling({text}) { + if (suspend) { + throw promise; + } else { + return 'Hello'; + } + } + + const onEvent = jest.fn(); + const TestResponder = React.unstable_createResponder('TestEventResponder', { + targetEventTypes: ['click'], + onEvent, + }); + + function Button() { + let listener = React.unstable_useResponder(TestResponder, {}); + return Click me; + } + + function App() { + return ( +
+ +
+ ); + } + + suspend = false; + let finalHTML = ReactDOMServer.renderToString(); + let container = document.createElement('div'); + container.innerHTML = finalHTML; + + // We need this to be in the document since we'll dispatch events on it. + document.body.appendChild(container); + + let a = container.getElementsByTagName('a')[0]; + + // On the client we don't have all data yet but we want to start + // hydrating anyway. + suspend = true; + let root = ReactDOM.unstable_createRoot(container, {hydrate: true}); + root.render(); + Scheduler.unstable_flushAll(); + jest.runAllTimers(); + + // We're now partially hydrated. + a.click(); + // We should not have invoked the event yet because we're not + // yet hydrated. + expect(onEvent).toHaveBeenCalledTimes(0); + + // Resolving the promise so that rendering can complete. + suspend = false; + resolve(); + await promise; + + Scheduler.unstable_flushAll(); + jest.runAllTimers(); + + // TODO: With selective hydration the event should've been replayed + // but for now we'll have to issue it again. + act(() => { + a.click(); + }); + + expect(onEvent).toHaveBeenCalledTimes(1); + + document.body.removeChild(container); + }); }); diff --git a/packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js b/packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js index d093a11bf8ab..a533923d5b73 100644 --- a/packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js +++ b/packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js @@ -13,6 +13,7 @@ let React; let ReactDOM; let ReactDOMServer; let Scheduler; +let act; // These tests rely both on ReactDOMServer and ReactDOM. // If a test only needs ReactDOMServer, put it in ReactServerRendering-test instead. @@ -23,6 +24,7 @@ describe('ReactDOMServerHydration', () => { ReactDOM = require('react-dom'); ReactDOMServer = require('react-dom/server'); Scheduler = require('scheduler'); + act = require('react-dom/test-utils').act; }); it('should have the correct mounting behavior (old hydrate API)', () => { @@ -499,4 +501,89 @@ describe('ReactDOMServerHydration', () => { Scheduler.unstable_flushAll(); expect(element.textContent).toBe('Hello world'); }); + + it('does not invoke an event on a concurrent hydrating node until it commits', () => { + function Sibling({text}) { + Scheduler.unstable_yieldValue('Sibling'); + return Sibling; + } + + function Sibling2({text}) { + Scheduler.unstable_yieldValue('Sibling2'); + return null; + } + + let clicks = 0; + + function Button() { + Scheduler.unstable_yieldValue('Button'); + let [clicked, setClicked] = React.useState(false); + if (clicked) { + return null; + } + return ( + { + setClicked(true); + clicks++; + }}> + Click me + + ); + } + + function App() { + return ( +
+
+ ); + } + + let finalHTML = ReactDOMServer.renderToString(); + let container = document.createElement('div'); + container.innerHTML = finalHTML; + expect(Scheduler).toHaveYielded(['Button', 'Sibling', 'Sibling2']); + + // We need this to be in the document since we'll dispatch events on it. + document.body.appendChild(container); + + let a = container.getElementsByTagName('a')[0]; + + // Hydrate asynchronously. + let root = ReactDOM.unstable_createRoot(container, {hydrate: true}); + root.render(); + // Flush part way through the render. + if (__DEV__) { + // In DEV effects gets double invoked. + expect(Scheduler).toFlushAndYieldThrough(['Button', 'Button', 'Sibling']); + } else { + expect(Scheduler).toFlushAndYieldThrough(['Button', 'Sibling']); + } + + expect(container.textContent).toBe('Click meSibling'); + + // We're now partially hydrated. + a.click(); + // Clicking should not invoke the event yet because we haven't committed + // the hydration yet. + expect(clicks).toBe(0); + + // Finish the rest of the hydration. + expect(Scheduler).toFlushAndYield(['Sibling2']); + + // TODO: With selective hydration the event should've been replayed + // but for now we'll have to issue it again. + act(() => { + a.click(); + }); + + expect(clicks).toBe(1); + + expect(container.textContent).toBe('Sibling'); + + document.body.removeChild(container); + }); }); diff --git a/packages/react-dom/src/client/ReactDOMComponentTree.js b/packages/react-dom/src/client/ReactDOMComponentTree.js index a3b61d6e49e0..8d3105ae443f 100644 --- a/packages/react-dom/src/client/ReactDOMComponentTree.js +++ b/packages/react-dom/src/client/ReactDOMComponentTree.js @@ -23,26 +23,29 @@ export function precacheFiberNode(hostInst, node) { * ReactDOMTextComponent instance ancestor. */ export function getClosestInstanceFromNode(node) { - if (node[internalInstanceKey]) { - return node[internalInstanceKey]; + let inst = node[internalInstanceKey]; + if (inst) { + return inst; } - while (!node[internalInstanceKey]) { - if (node.parentNode) { - node = node.parentNode; + do { + node = node.parentNode; + if (node) { + inst = node[internalInstanceKey]; } else { // Top of the tree. This node must not be part of a React tree (or is // unmounted, potentially). return null; } - } + } while (!inst); - let inst = node[internalInstanceKey]; - if (inst.tag === HostComponent || inst.tag === HostText) { - // In Fiber, this will always be the deepest root. - return inst; + let tag = inst.tag; + switch (tag) { + case HostComponent: + case HostText: + // In Fiber, this will always be the deepest root. + return inst; } - return null; } diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index a28c09a6baf0..4ece9c03d25c 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -46,6 +46,7 @@ import { NoEffect, PerformedWork, Placement, + Hydrating, ContentReset, DidCapture, Update, @@ -944,11 +945,10 @@ function updateHostRoot(current, workInProgress, renderExpirationTime) { // be any children to hydrate which is effectively the same thing as // not hydrating. - // This is a bit of a hack. We track the host root as a placement to - // know that we're currently in a mounting state. That way isMounted - // works as expected. We must reset this before committing. - // TODO: Delete this when we delete isMounted and findDOMNode. - workInProgress.effectTag |= Placement; + // Mark the host root with a Hydrating effect to know that we're + // currently in a mounting state. That way isMounted, findDOMNode and + // event replaying works as expected. + workInProgress.effectTag |= Hydrating; // Ensure that children mount into this root without tracking // side-effects. This ensures that we don't store Placement effects on @@ -2095,12 +2095,24 @@ function updateDehydratedSuspenseComponent( ); const nextProps = workInProgress.pendingProps; const nextChildren = nextProps.children; - workInProgress.child = mountChildFibers( + const child = mountChildFibers( workInProgress, null, nextChildren, renderExpirationTime, ); + let node = child; + while (node) { + // Mark each child as hydrating. This is a fast path to know whether this + // tree is part of a hydrating tree. This is used to determine if a child + // node has fully mounted yet, and for scheduling event replaying. + // Conceptually this is similar to Placement in that a new subtree is + // inserted into the React tree here. It just happens to not need DOM + // mutations because it already exists. + node.effectTag |= Hydrating; + node = node.sibling; + } + workInProgress.child = child; return workInProgress.child; } } diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 6d5fa4f3173f..73d67d0e880a 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -56,7 +56,6 @@ import { } from 'shared/ReactWorkTags'; import {NoMode, BatchedMode} from './ReactTypeOfMode'; import { - Placement, Ref, Update, NoEffect, @@ -670,9 +669,6 @@ function completeWork( // If we hydrated, pop so that we can delete any remaining children // that weren't hydrated. popHydrationState(workInProgress); - // This resets the hacky state to fix isMounted before committing. - // TODO: Delete this when we delete isMounted and findDOMNode. - workInProgress.effectTag &= ~Placement; } updateHostContainer(workInProgress); break; @@ -859,14 +855,13 @@ function completeWork( if ((workInProgress.effectTag & DidCapture) === NoEffect) { // This boundary did not suspend so it's now hydrated and unsuspended. workInProgress.memoizedState = null; - if (enableSuspenseCallback) { - // Notify the callback. - workInProgress.effectTag |= Update; - } - } else { - // Something suspended. Schedule an effect to attach retry listeners. - workInProgress.effectTag |= Update; } + // If nothing suspended, we need to schedule an effect to mark this boundary + // as having hydrated so events know that they're free be invoked. + // It's also a signal to replay events and the suspense callback. + // If something suspended, schedule an effect to attach retry listeners. + // So we might as well always mark this. + workInProgress.effectTag |= Update; return null; } } diff --git a/packages/react-reconciler/src/ReactFiberTreeReflection.js b/packages/react-reconciler/src/ReactFiberTreeReflection.js index c10762f05151..7b9e687de69f 100644 --- a/packages/react-reconciler/src/ReactFiberTreeReflection.js +++ b/packages/react-reconciler/src/ReactFiberTreeReflection.js @@ -23,7 +23,7 @@ import { HostText, FundamentalComponent, } from 'shared/ReactWorkTags'; -import {NoEffect, Placement} from 'shared/ReactSideEffectTags'; +import {NoEffect, Placement, Hydrating} from 'shared/ReactSideEffectTags'; import {enableFundamentalAPI} from 'shared/ReactFeatureFlags'; const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner; @@ -32,20 +32,21 @@ const MOUNTING = 1; const MOUNTED = 2; const UNMOUNTED = 3; -function isFiberMountedImpl(fiber: Fiber): number { +type MountState = 1 | 2 | 3; + +function isFiberMountedImpl(fiber: Fiber): MountState { let node = fiber; if (!fiber.alternate) { // If there is no alternate, this might be a new tree that isn't inserted // yet. If it is, then it will have a pending insertion effect on it. - if ((node.effectTag & Placement) !== NoEffect) { - return MOUNTING; - } - while (node.return) { - node = node.return; - if ((node.effectTag & Placement) !== NoEffect) { + let nextNode = node; + do { + node = nextNode; + if ((node.effectTag & (Placement | Hydrating)) !== NoEffect) { return MOUNTING; } - } + nextNode = node.return; + } while (nextNode); } else { while (node.return) { node = node.return; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 21454b8feeba..9d9dad28a41a 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -96,6 +96,8 @@ import { Passive, Incomplete, HostEffectMask, + Hydrating, + HydratingAndUpdate, } from 'shared/ReactSideEffectTags'; import { NoWork, @@ -1860,7 +1862,8 @@ function commitMutationEffects(root: FiberRoot, renderPriorityLevel) { // updates, and deletions. To avoid needing to add a case for every possible // bitmap value, we remove the secondary effects from the effect tag and // switch on that value. - let primaryEffectTag = effectTag & (Placement | Update | Deletion); + let primaryEffectTag = + effectTag & (Placement | Update | Deletion | Hydrating); switch (primaryEffectTag) { case Placement: { commitPlacement(nextEffect); @@ -1883,6 +1886,18 @@ function commitMutationEffects(root: FiberRoot, renderPriorityLevel) { commitWork(current, nextEffect); break; } + case Hydrating: { + nextEffect.effectTag &= ~Hydrating; + break; + } + case HydratingAndUpdate: { + nextEffect.effectTag &= ~Hydrating; + + // Update + const current = nextEffect.alternate; + commitWork(current, nextEffect); + break; + } case Update: { const current = nextEffect.alternate; commitWork(current, nextEffect); diff --git a/packages/shared/ReactSideEffectTags.js b/packages/shared/ReactSideEffectTags.js index 5f3c47f8fd59..ad3f01ac4b93 100644 --- a/packages/shared/ReactSideEffectTags.js +++ b/packages/shared/ReactSideEffectTags.js @@ -10,26 +10,28 @@ export type SideEffectTag = number; // Don't change these two values. They're used by React Dev Tools. -export const NoEffect = /* */ 0b000000000000; -export const PerformedWork = /* */ 0b000000000001; +export const NoEffect = /* */ 0b0000000000000; +export const PerformedWork = /* */ 0b0000000000001; // You can change the rest (and add more). -export const Placement = /* */ 0b000000000010; -export const Update = /* */ 0b000000000100; -export const PlacementAndUpdate = /* */ 0b000000000110; -export const Deletion = /* */ 0b000000001000; -export const ContentReset = /* */ 0b000000010000; -export const Callback = /* */ 0b000000100000; -export const DidCapture = /* */ 0b000001000000; -export const Ref = /* */ 0b000010000000; -export const Snapshot = /* */ 0b000100000000; -export const Passive = /* */ 0b001000000000; +export const Placement = /* */ 0b0000000000010; +export const Update = /* */ 0b0000000000100; +export const PlacementAndUpdate = /* */ 0b0000000000110; +export const Deletion = /* */ 0b0000000001000; +export const ContentReset = /* */ 0b0000000010000; +export const Callback = /* */ 0b0000000100000; +export const DidCapture = /* */ 0b0000001000000; +export const Ref = /* */ 0b0000010000000; +export const Snapshot = /* */ 0b0000100000000; +export const Passive = /* */ 0b0001000000000; +export const Hydrating = /* */ 0b0010000000000; +export const HydratingAndUpdate = /* */ 0b0010000000100; // Passive & Update & Callback & Ref & Snapshot -export const LifecycleEffectMask = /* */ 0b001110100100; +export const LifecycleEffectMask = /* */ 0b0001110100100; // Union of all host effects -export const HostEffectMask = /* */ 0b001111111111; +export const HostEffectMask = /* */ 0b0011111111111; -export const Incomplete = /* */ 0b010000000000; -export const ShouldCapture = /* */ 0b100000000000; +export const Incomplete = /* */ 0b0100000000000; +export const ShouldCapture = /* */ 0b1000000000000; From 16c34086380fc4e4ff20fca1ed27e9e35bd98299 Mon Sep 17 00:00:00 2001 From: Bruno Scopelliti Date: Thu, 22 Aug 2019 18:31:27 +0200 Subject: [PATCH 08/13] Only warn in case the fourth argument is a function (#16543) --- packages/react-reconciler/src/ReactFiberHooks.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 90b7d82eef56..5c36e6407032 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -1122,7 +1122,7 @@ function dispatchAction( if (__DEV__) { warning( - arguments.length <= 3, + typeof arguments[3] !== 'function', "State updates from the useState() and useReducer() Hooks don't support the " + 'second callback argument. To execute a side effect after ' + 'rendering, declare it in the component body with useEffect().', From 2f03aa6eed14ae30e586b77e40b2da9ea5e6facc Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Thu, 22 Aug 2019 10:22:14 -0700 Subject: [PATCH 09/13] [react-events] Fix middle-click for Press (#16546) Browsers always report 'buttons' as 0 when a pointer is released. --- packages/react-events/src/dom/Press.js | 22 ++++++++----------- .../src/dom/__tests__/Press-test.internal.js | 6 ++--- .../src/dom/testing-library/domEvents.js | 4 ++-- 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/packages/react-events/src/dom/Press.js b/packages/react-events/src/dom/Press.js index 3a08adc280c2..2b4858d3746c 100644 --- a/packages/react-events/src/dom/Press.js +++ b/packages/react-events/src/dom/Press.js @@ -43,6 +43,7 @@ type PressState = { y: number, |}>, addedRootEvents: boolean, + buttons: 0 | 1 | 4, isActivePressed: boolean, isActivePressStart: boolean, isPressed: boolean, @@ -149,9 +150,9 @@ function createPressEvent( event: ?ReactDOMResponderEvent, touchEvent: null | Touch, defaultPrevented: boolean, + state: PressState, ): PressEvent { const timeStamp = context.getTimeStamp(); - let buttons = 1; let clientX = null; let clientY = null; let pageX = null; @@ -171,20 +172,12 @@ function createPressEvent( let eventObject; eventObject = (touchEvent: any) || (nativeEvent: any); if (eventObject) { - ({ - buttons, - clientX, - clientY, - pageX, - pageY, - screenX, - screenY, - } = eventObject); + ({clientX, clientY, pageX, pageY, screenX, screenY} = eventObject); } } return { altKey, - buttons, + buttons: state.buttons, clientX, clientY, ctrlKey, @@ -226,6 +219,7 @@ function dispatchEvent( event, touchEvent, defaultPrevented, + state, ); context.dispatchEvent(syntheticEvent, listener, eventPriority); } @@ -493,6 +487,7 @@ const pressResponderImpl = { return { activationPosition: null, addedRootEvents: false, + buttons: 0, isActivePressed: false, isActivePressStart: false, isPressed: false, @@ -586,7 +581,7 @@ const pressResponderImpl = { state.activePointerId = touchEvent.identifier; } - // Ignore any device buttons except primary/secondary and touch/pen contact. + // Ignore any device buttons except primary/middle and touch/pen contact. // Additionally we ignore primary-button + ctrl-key with Macs as that // acts like right-click and opens the contextmenu. if ( @@ -606,6 +601,7 @@ const pressResponderImpl = { } state.responderRegionOnDeactivation = null; state.isPressWithinResponderRegion = true; + state.buttons = nativeEvent.buttons; dispatchPressStartEvents(event, context, props, state); addRootEventTypes(context, state); } else { @@ -709,7 +705,7 @@ const pressResponderImpl = { case 'mouseup': case 'touchend': { if (isPressed) { - const buttons = nativeEvent.buttons; + const buttons = state.buttons; let isKeyboardEvent = false; let touchEvent; if (type === 'pointerup' && activePointerId !== pointerId) { diff --git a/packages/react-events/src/dom/__tests__/Press-test.internal.js b/packages/react-events/src/dom/__tests__/Press-test.internal.js index 34e483e5a54c..ea4aa15aaa81 100644 --- a/packages/react-events/src/dom/__tests__/Press-test.internal.js +++ b/packages/react-events/src/dom/__tests__/Press-test.internal.js @@ -136,7 +136,7 @@ describe.each(environmentTable)('Press responder', hasPointerEvents => { const target = createEventTarget(node); target.setBoundingClientRect({x: 0, y: 0, width: 100, height: 100}); target.pointerdown({buttons: buttonsType.middle, pointerType: 'mouse'}); - target.pointerup({buttons: buttonsType.middle, pointerType: 'mouse'}); + target.pointerup({pointerType: 'mouse'}); target.pointerhover({x: 110, y: 110}); target.pointerhover({x: 50, y: 50}); expect(onPressStart).toHaveBeenCalledTimes(1); @@ -216,7 +216,7 @@ describe.each(environmentTable)('Press responder', hasPointerEvents => { it('is called after middle-button pointer up', () => { const target = createEventTarget(ref.current); target.pointerdown({buttons: buttonsType.middle, pointerType: 'mouse'}); - target.pointerup({buttons: buttonsType.middle, pointerType: 'mouse'}); + target.pointerup({pointerType: 'mouse'}); expect(onPressEnd).toHaveBeenCalledTimes(1); expect(onPressEnd).toHaveBeenCalledWith( expect.objectContaining({ @@ -357,7 +357,7 @@ describe.each(environmentTable)('Press responder', hasPointerEvents => { it('is not called after middle-button press', () => { const target = createEventTarget(ref.current); target.pointerdown({buttons: buttonsType.middle, pointerType: 'mouse'}); - target.pointerup({buttons: buttonsType.middle, pointerType: 'mouse'}); + target.pointerup({pointerType: 'mouse'}); expect(onPress).not.toHaveBeenCalled(); }); diff --git a/packages/react-events/src/dom/testing-library/domEvents.js b/packages/react-events/src/dom/testing-library/domEvents.js index 288884531f4f..010b0c345ad8 100644 --- a/packages/react-events/src/dom/testing-library/domEvents.js +++ b/packages/react-events/src/dom/testing-library/domEvents.js @@ -329,8 +329,8 @@ export function pointerover(payload) { export function pointerup(payload) { return createPointerEvent('pointerup', { - buttons: buttonsType.none, ...payload, + buttons: buttonsType.none, }); } @@ -367,8 +367,8 @@ export function mouseover(payload) { export function mouseup(payload) { return createMouseEvent('mouseup', { - buttons: buttonsType.none, ...payload, + buttons: buttonsType.none, }); } From 474b650cac72c9a251fa4a9f8d8d121dff067970 Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Thu, 22 Aug 2019 13:30:35 -0700 Subject: [PATCH 10/13] [react-events] Rename hook exports (#16533) For example, 'useHoverResponder' becomes 'useHover' --- packages/react-events/docs/ContextMenu.md | 54 ++++++++++++++ packages/react-events/docs/Focus.md | 32 ++++---- packages/react-events/docs/FocusWithin.md | 41 ++++------ packages/react-events/docs/Hover.md | 48 +++++++----- packages/react-events/docs/Press.md | 38 +++++----- packages/react-events/src/dom/ContextMenu.js | 2 +- packages/react-events/src/dom/Drag.js | 2 +- packages/react-events/src/dom/Focus.js | 4 +- packages/react-events/src/dom/Hover.js | 2 +- packages/react-events/src/dom/Input.js | 2 +- packages/react-events/src/dom/Keyboard.js | 2 +- packages/react-events/src/dom/Press.js | 2 +- packages/react-events/src/dom/Scroll.js | 2 +- packages/react-events/src/dom/Swipe.js | 4 +- .../__tests__/ContextMenu-test.internal.js | 17 ++--- .../src/dom/__tests__/Drag-test.internal.js | 10 +-- .../src/dom/__tests__/Focus-test.internal.js | 18 ++--- .../__tests__/FocusWithin-test.internal.js | 11 ++- .../src/dom/__tests__/Hover-test.internal.js | 20 ++--- .../src/dom/__tests__/Input-test.internal.js | 50 ++++++------- .../dom/__tests__/Keyboard-test.internal.js | 10 +-- .../src/dom/__tests__/Press-test.internal.js | 74 +++++++++---------- .../src/dom/__tests__/Scroll-test.internal.js | 12 +-- packages/react-events/src/rn/Press.js | 2 +- 24 files changed, 259 insertions(+), 200 deletions(-) create mode 100644 packages/react-events/docs/ContextMenu.md diff --git a/packages/react-events/docs/ContextMenu.md b/packages/react-events/docs/ContextMenu.md new file mode 100644 index 000000000000..33b95c452be3 --- /dev/null +++ b/packages/react-events/docs/ContextMenu.md @@ -0,0 +1,54 @@ +# ContextMenu + +The `useContextMenu` hooks responds to context-menu events. + +```js +// Example +const Button = (props) => { + const contextmenu = useContextMenu({ + disabled, + onContextMenu, + preventDefault + }); + + return ( +
+ {props.children} +
+ ); +}; +``` + +## Types + +```js +type ContextMenuEvent = { + altKey: boolean, + buttons: 0 | 1 | 2, + ctrlKey: boolean, + metaKey: boolean, + pageX: number, + pageY: number, + pointerType: PointerType, + shiftKey: boolean, + target: Element, + timeStamp: number, + type: 'contextmenu', + x: number, + y: number, +} +``` + +## Props + +### disabled: boolean = false + +Disables the responder. + +### onContextMenu: (e: ContextMenuEvent) => void + +Called when the user performs a gesture to display a context menu. + +### preventDefault: boolean = true + +Prevents the native behavior (i.e., context menu). diff --git a/packages/react-events/docs/Focus.md b/packages/react-events/docs/Focus.md index 1fef78e370ff..47afc8724003 100644 --- a/packages/react-events/docs/Focus.md +++ b/packages/react-events/docs/Focus.md @@ -1,29 +1,29 @@ # Focus -The `Focus` module responds to focus and blur events on its child. Focus events +The `useFocus` hook responds to focus and blur events on its child. Focus events are dispatched for all input types, with the exception of `onFocusVisibleChange` which is only dispatched when focusing with a keyboard. -Focus events do not propagate between `Focus` event responders. +Focus events do not propagate between `useFocus` event responders. ```js // Example const Button = (props) => { - const [ focusVisible, setFocusVisible ] = useState(false); + const [ isFocusVisible, setFocusVisible ] = useState(false); + const focus = useFocus({ + onBlur={props.onBlur} + onFocus={props.onFocus} + onFocusVisibleChange={setFocusVisible} + }); return ( - - ; + return ( + + ); }; expect(() => { handler = event => { event.preventDefault(); }; ReactDOM.render(, container); - dispatchClickEvent(document.body); + dispatchClickEvent(buttonRef.current); }).toWarnDev( 'Warning: preventDefault() is not available on event objects created from event responder modules ' + '(React Flare).' + @@ -847,7 +854,7 @@ describe('DOMEventResponderSystem', () => { event.stopPropagation(); }; ReactDOM.render(, container); - dispatchClickEvent(document.body); + dispatchClickEvent(buttonRef.current); }).toWarnDev( 'Warning: stopPropagation() is not available on event objects created from event responder modules ' + '(React Flare).' + @@ -859,7 +866,7 @@ describe('DOMEventResponderSystem', () => { event.isDefaultPrevented(); }; ReactDOM.render(, container); - dispatchClickEvent(document.body); + dispatchClickEvent(buttonRef.current); }).toWarnDev( 'Warning: isDefaultPrevented() is not available on event objects created from event responder modules ' + '(React Flare).' + @@ -871,7 +878,7 @@ describe('DOMEventResponderSystem', () => { event.isPropagationStopped(); }; ReactDOM.render(, container); - dispatchClickEvent(document.body); + dispatchClickEvent(buttonRef.current); }).toWarnDev( 'Warning: isPropagationStopped() is not available on event objects created from event responder modules ' + '(React Flare).' + @@ -883,7 +890,7 @@ describe('DOMEventResponderSystem', () => { return event.nativeEvent; }; ReactDOM.render(, container); - dispatchClickEvent(document.body); + dispatchClickEvent(buttonRef.current); }).toWarnDev( 'Warning: nativeEvent is not available on event objects created from event responder modules ' + '(React Flare).' + @@ -934,4 +941,57 @@ describe('DOMEventResponderSystem', () => { ReactDOM.render(, container); buttonRef.current.dispatchEvent(createEvent('foobar')); }); + + it('should work with concurrent mode updates', async () => { + const log = []; + const TestResponder = createEventResponder({ + targetEventTypes: ['click'], + onEvent(event, context, props) { + log.push(props); + }, + }); + const ref = React.createRef(); + + function Test({counter}) { + const listener = React.unstable_useResponder(TestResponder, {counter}); + + return ( + + ); + } + + let root = ReactDOM.unstable_createRoot(container); + let batch = root.createBatch(); + batch.render(); + Scheduler.unstable_flushAll(); + jest.runAllTimers(); + batch.commit(); + + // Click the button + dispatchClickEvent(ref.current); + expect(log).toEqual([{counter: 0}]); + + // Clear log + log.length = 0; + + // Increase counter + batch = root.createBatch(); + batch.render(); + Scheduler.unstable_flushAll(); + jest.runAllTimers(); + + // Click the button again + dispatchClickEvent(ref.current); + expect(log).toEqual([{counter: 0}]); + + // Clear log + log.length = 0; + + // Commit + batch.commit(); + dispatchClickEvent(ref.current); + expect(log).toEqual([{counter: 1}]); + }); }); diff --git a/packages/react-native-renderer/src/ReactFabricHostConfig.js b/packages/react-native-renderer/src/ReactFabricHostConfig.js index 412e7b1094d2..977abafa9b49 100644 --- a/packages/react-native-renderer/src/ReactFabricHostConfig.js +++ b/packages/react-native-renderer/src/ReactFabricHostConfig.js @@ -449,7 +449,6 @@ export function mountResponderInstance( props: Object, state: Object, instance: Instance, - rootContainerInstance: Container, ) { if (enableFlareAPI) { const {rootEventTypes} = responder; diff --git a/packages/react-native-renderer/src/ReactNativeHostConfig.js b/packages/react-native-renderer/src/ReactNativeHostConfig.js index 8f06dff0308f..dd9e7e0e08a1 100644 --- a/packages/react-native-renderer/src/ReactNativeHostConfig.js +++ b/packages/react-native-renderer/src/ReactNativeHostConfig.js @@ -501,7 +501,6 @@ export function mountResponderInstance( props: Object, state: Object, instance: Instance, - rootContainerInstance: Container, ) { throw new Error('Not yet implemented.'); } diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index b8996ad698ee..072e0fa8c8ed 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -118,6 +118,7 @@ import { } from './ReactHookEffectTags'; import {didWarnAboutReassigningProps} from './ReactFiberBeginWork'; import {runWithPriority, NormalPriority} from './SchedulerWithReactIntegration'; +import {updateEventListeners} from './ReactFiberEvents'; let didWarnAboutUndefinedSnapshotBeforeUpdate: Set | null = null; if (__DEV__) { @@ -1331,6 +1332,13 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void { finishedWork, ); } + if (enableFlareAPI) { + const prevListeners = oldProps.listeners; + const nextListeners = newProps.listeners; + if (prevListeners !== nextListeners) { + updateEventListeners(nextListeners, instance, finishedWork); + } + } } return; } diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 73d67d0e880a..f77eb30bf6aa 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -9,12 +9,7 @@ import type {Fiber} from './ReactFiber'; import type {ExpirationTime} from './ReactFiberExpirationTime'; -import type { - ReactEventResponder, - ReactEventResponderInstance, - ReactFundamentalComponentInstance, - ReactEventResponderListener, -} from 'shared/ReactTypes'; +import type {ReactFundamentalComponentInstance} from 'shared/ReactTypes'; import type {FiberRoot} from './ReactFiberRoot'; import type { Instance, @@ -31,7 +26,6 @@ import type {SuspenseContext} from './ReactFiberSuspenseContext'; import {now} from './SchedulerWithReactIntegration'; -import {REACT_RESPONDER_TYPE} from 'shared/ReactSymbols'; import { IndeterminateComponent, FunctionComponent, @@ -78,8 +72,6 @@ import { createContainerChildSet, appendChildToContainerChildSet, finalizeContainerChildren, - mountResponderInstance, - unmountResponderInstance, getFundamentalComponentInstance, mountFundamentalComponent, cloneFundamentalInstance, @@ -91,8 +83,6 @@ import { getHostContext, popHostContainer, } from './ReactFiberHostContext'; -import {NoWork} from './ReactFiberExpirationTime'; -import {createResponderInstance} from './ReactFiberEvents'; import { suspenseStackCursor, InvisibleParentSuspenseContext, @@ -132,10 +122,7 @@ import { import {createFundamentalStateInstance} from './ReactFiberFundamental'; import {Never} from './ReactFiberExpirationTime'; import {resetChildFibers} from './ReactChildFiber'; -import warning from 'shared/warning'; - -const emptyObject = {}; -const isArray = Array.isArray; +import {updateEventListeners} from './ReactFiberEvents'; function markUpdate(workInProgress: Fiber) { // Tag the fiber with an update effect. This turns a Placement into @@ -689,14 +676,8 @@ function completeWork( if (enableFlareAPI) { const prevListeners = current.memoizedProps.listeners; const nextListeners = newProps.listeners; - const instance = workInProgress.stateNode; if (prevListeners !== nextListeners) { - updateEventListeners( - nextListeners, - instance, - rootContainerInstance, - workInProgress, - ); + markUpdate(workInProgress); } } @@ -738,12 +719,7 @@ function completeWork( const instance = workInProgress.stateNode; const listeners = newProps.listeners; if (listeners != null) { - updateEventListeners( - listeners, - instance, - rootContainerInstance, - workInProgress, - ); + updateEventListeners(listeners, instance, workInProgress); } } } else { @@ -760,12 +736,7 @@ function completeWork( if (enableFlareAPI) { const listeners = newProps.listeners; if (listeners != null) { - updateEventListeners( - listeners, - instance, - rootContainerInstance, - workInProgress, - ); + updateEventListeners(listeners, instance, workInProgress); } } @@ -1253,156 +1224,4 @@ function completeWork( return null; } -function mountEventResponder( - responder: ReactEventResponder, - responderProps: Object, - instance: Instance, - rootContainerInstance: Container, - fiber: Fiber, - respondersMap: Map< - ReactEventResponder, - ReactEventResponderInstance, - >, -) { - let responderState = emptyObject; - const getInitialState = responder.getInitialState; - if (getInitialState !== null) { - responderState = getInitialState(responderProps); - } - const responderInstance = createResponderInstance( - responder, - responderProps, - responderState, - instance, - fiber, - ); - mountResponderInstance( - responder, - responderInstance, - responderProps, - responderState, - instance, - rootContainerInstance, - ); - respondersMap.set(responder, responderInstance); -} - -function updateEventListener( - listener: ReactEventResponderListener, - fiber: Fiber, - visistedResponders: Set>, - respondersMap: Map< - ReactEventResponder, - ReactEventResponderInstance, - >, - instance: Instance, - rootContainerInstance: Container, -): void { - let responder; - let props; - - if (listener) { - responder = listener.responder; - props = listener.props; - } - invariant( - responder && responder.$$typeof === REACT_RESPONDER_TYPE, - 'An invalid value was used as an event listener. Expect one or many event ' + - 'listeners created via React.unstable_useResponder().', - ); - const listenerProps = ((props: any): Object); - if (visistedResponders.has(responder)) { - // show warning - if (__DEV__) { - warning( - false, - 'Duplicate event responder "%s" found in event listeners. ' + - 'Event listeners passed to elements cannot use the same event responder more than once.', - responder.displayName, - ); - } - return; - } - visistedResponders.add(responder); - const responderInstance = respondersMap.get(responder); - - if (responderInstance === undefined) { - // Mount - mountEventResponder( - responder, - listenerProps, - instance, - rootContainerInstance, - fiber, - respondersMap, - ); - } else { - // Update - responderInstance.props = listenerProps; - responderInstance.fiber = fiber; - } -} - -function updateEventListeners( - listeners: any, - instance: Instance, - rootContainerInstance: Container, - fiber: Fiber, -): void { - const visistedResponders = new Set(); - let dependencies = fiber.dependencies; - if (listeners != null) { - if (dependencies === null) { - dependencies = fiber.dependencies = { - expirationTime: NoWork, - firstContext: null, - responders: new Map(), - }; - } - let respondersMap = dependencies.responders; - if (respondersMap === null) { - respondersMap = new Map(); - } - if (isArray(listeners)) { - for (let i = 0, length = listeners.length; i < length; i++) { - const listener = listeners[i]; - updateEventListener( - listener, - fiber, - visistedResponders, - respondersMap, - instance, - rootContainerInstance, - ); - } - } else { - updateEventListener( - listeners, - fiber, - visistedResponders, - respondersMap, - instance, - rootContainerInstance, - ); - } - } - if (dependencies !== null) { - const respondersMap = dependencies.responders; - if (respondersMap !== null) { - // Unmount - const mountedResponders = Array.from(respondersMap.keys()); - for (let i = 0, length = mountedResponders.length; i < length; i++) { - const mountedResponder = mountedResponders[i]; - if (!visistedResponders.has(mountedResponder)) { - const responderInstance = ((respondersMap.get( - mountedResponder, - ): any): ReactEventResponderInstance); - unmountResponderInstance(responderInstance); - respondersMap.delete(mountedResponder); - } - } - } - } -} - export {completeWork}; diff --git a/packages/react-reconciler/src/ReactFiberEvents.js b/packages/react-reconciler/src/ReactFiberEvents.js index a3f4404140d9..b87a11eb2ed0 100644 --- a/packages/react-reconciler/src/ReactFiberEvents.js +++ b/packages/react-reconciler/src/ReactFiberEvents.js @@ -8,59 +8,26 @@ */ import type {Fiber} from './ReactFiber'; +import type {Instance} from './ReactFiberHostConfig'; import type { ReactEventResponder, ReactEventResponderInstance, ReactEventResponderListener, } from 'shared/ReactTypes'; -import type {Instance} from './ReactFiberHostConfig'; -import {SuspenseComponent, Fragment} from 'shared/ReactWorkTags'; +import { + mountResponderInstance, + unmountResponderInstance, +} from './ReactFiberHostConfig'; +import {NoWork} from './ReactFiberExpirationTime'; -export function createResponderListener( - responder: ReactEventResponder, - props: Object, -): ReactEventResponderListener { - const eventResponderListener = { - responder, - props, - }; - if (__DEV__) { - Object.freeze(eventResponderListener); - } - return eventResponderListener; -} +import warning from 'shared/warning'; +import {REACT_RESPONDER_TYPE} from 'shared/ReactSymbols'; -export function isFiberSuspenseAndTimedOut(fiber: Fiber): boolean { - return fiber.tag === SuspenseComponent && fiber.memoizedState !== null; -} +import invariant from 'shared/invariant'; -export function getSuspenseFallbackChild(fiber: Fiber): Fiber | null { - return ((((fiber.child: any): Fiber).sibling: any): Fiber).child; -} - -export function isFiberSuspenseTimedOutChild(fiber: Fiber | null): boolean { - if (fiber === null) { - return false; - } - const parent = fiber.return; - if (parent !== null && parent.tag === Fragment) { - const grandParent = parent.return; - - if ( - grandParent !== null && - grandParent.tag === SuspenseComponent && - grandParent.stateNode !== null - ) { - return true; - } - } - return false; -} - -export function getSuspenseFiberFromTimedOutChild(fiber: Fiber): Fiber { - return ((((fiber.return: any): Fiber).return: any): Fiber); -} +const emptyObject = {}; +const isArray = Array.isArray; export function createResponderInstance( responder: ReactEventResponder, @@ -78,3 +45,162 @@ export function createResponderInstance( target, }; } + +function mountEventResponder( + responder: ReactEventResponder, + responderProps: Object, + instance: Instance, + fiber: Fiber, + respondersMap: Map< + ReactEventResponder, + ReactEventResponderInstance, + >, +) { + let responderState = emptyObject; + const getInitialState = responder.getInitialState; + if (getInitialState !== null) { + responderState = getInitialState(responderProps); + } + const responderInstance = createResponderInstance( + responder, + responderProps, + responderState, + instance, + fiber, + ); + mountResponderInstance( + responder, + responderInstance, + responderProps, + responderState, + instance, + ); + respondersMap.set(responder, responderInstance); +} + +function updateEventListener( + listener: ReactEventResponderListener, + fiber: Fiber, + visistedResponders: Set>, + respondersMap: Map< + ReactEventResponder, + ReactEventResponderInstance, + >, + instance: Instance, +): void { + let responder; + let props; + + if (listener) { + responder = listener.responder; + props = listener.props; + } + invariant( + responder && responder.$$typeof === REACT_RESPONDER_TYPE, + 'An invalid value was used as an event listener. Expect one or many event ' + + 'listeners created via React.unstable_useResponder().', + ); + const listenerProps = ((props: any): Object); + if (visistedResponders.has(responder)) { + // show warning + if (__DEV__) { + warning( + false, + 'Duplicate event responder "%s" found in event listeners. ' + + 'Event listeners passed to elements cannot use the same event responder more than once.', + responder.displayName, + ); + } + return; + } + visistedResponders.add(responder); + const responderInstance = respondersMap.get(responder); + + if (responderInstance === undefined) { + // Mount (happens in either complete or commit phase) + mountEventResponder( + responder, + listenerProps, + instance, + fiber, + respondersMap, + ); + } else { + // Update (happens during commit phase only) + responderInstance.props = listenerProps; + responderInstance.fiber = fiber; + } +} + +export function updateEventListeners( + listeners: any, + instance: Instance, + fiber: Fiber, +): void { + const visistedResponders = new Set(); + let dependencies = fiber.dependencies; + if (listeners != null) { + if (dependencies === null) { + dependencies = fiber.dependencies = { + expirationTime: NoWork, + firstContext: null, + responders: new Map(), + }; + } + let respondersMap = dependencies.responders; + if (respondersMap === null) { + respondersMap = new Map(); + } + if (isArray(listeners)) { + for (let i = 0, length = listeners.length; i < length; i++) { + const listener = listeners[i]; + updateEventListener( + listener, + fiber, + visistedResponders, + respondersMap, + instance, + ); + } + } else { + updateEventListener( + listeners, + fiber, + visistedResponders, + respondersMap, + instance, + ); + } + } + if (dependencies !== null) { + const respondersMap = dependencies.responders; + if (respondersMap !== null) { + // Unmount + const mountedResponders = Array.from(respondersMap.keys()); + for (let i = 0, length = mountedResponders.length; i < length; i++) { + const mountedResponder = mountedResponders[i]; + if (!visistedResponders.has(mountedResponder)) { + const responderInstance = ((respondersMap.get( + mountedResponder, + ): any): ReactEventResponderInstance); + unmountResponderInstance(responderInstance); + respondersMap.delete(mountedResponder); + } + } + } + } +} + +export function createResponderListener( + responder: ReactEventResponder, + props: Object, +): ReactEventResponderListener { + const eventResponderListener = { + responder, + props, + }; + if (__DEV__) { + Object.freeze(eventResponderListener); + } + return eventResponderListener; +} diff --git a/packages/react-test-renderer/src/ReactTestHostConfig.js b/packages/react-test-renderer/src/ReactTestHostConfig.js index 5f5de45dafd5..e6df01d0e726 100644 --- a/packages/react-test-renderer/src/ReactTestHostConfig.js +++ b/packages/react-test-renderer/src/ReactTestHostConfig.js @@ -296,7 +296,6 @@ export function mountResponderInstance( props: Object, state: Object, instance: Instance, - rootContainerInstance: Container, ) { // noop } From 0da7bd0604a5be7f96572b9f75d16fef5674bc5d Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 26 Aug 2019 09:34:06 -0700 Subject: [PATCH 13/13] React DevTools CHANGELOG entry for 4.0.6 --- packages/react-devtools/CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/react-devtools/CHANGELOG.md b/packages/react-devtools/CHANGELOG.md index bc10a248baca..894bc3c75a61 100644 --- a/packages/react-devtools/CHANGELOG.md +++ b/packages/react-devtools/CHANGELOG.md @@ -9,6 +9,11 @@ +## 4.0.6 (August 26, 2019) +#### Bug fixes +* Remove ⚛️ emoji prefix from Firefox extension tab labels +* Standalone polyfills `Symbol` usage + ## 4.0.5 (August 19, 2019) #### Bug fixes * Props, state, and context values are alpha sorted.