From de9824c2d5212e31d55c2e82c4bf267df7e59f14 Mon Sep 17 00:00:00 2001 From: Ben Alpert Date: Fri, 18 Nov 2016 23:20:32 +0000 Subject: [PATCH] Restore DOM selection and suppress events This makes Draft mostly work. --- scripts/fiber/tests-failing.txt | 3 - scripts/fiber/tests-passing.txt | 3 + src/renderers/dom/fiber/ReactDOMFiber.js | 21 +++- .../stack/client/__tests__/ReactDOM-test.js | 60 ++++++++++ src/renderers/noop/ReactNoop.js | 17 +++ .../shared/fiber/ReactFiberReconciler.js | 3 + .../shared/fiber/ReactFiberScheduler.js | 104 +++++++++++------- .../ReactIncrementalErrorHandling-test.js | 26 +++++ 8 files changed, 190 insertions(+), 47 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 23726bdeccb9a..cf51db15c69f5 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -85,9 +85,6 @@ src/renderers/dom/shared/eventPlugins/__tests__/SimpleEventPlugin-test.js * should not forward clicks when it becomes disabled * should work correctly if the listener is changed -src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js -* should control a value in reentrant events - src/renderers/dom/stack/client/__tests__/ReactDOM-test.js * throws in render() if the mount callback is not a function * throws in render() if the update callback is not a function diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 9a54b3c411b83..ca7e7aa5bad18 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -779,6 +779,7 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMIframe-test.js src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js * should properly control a value even if no event listener exists +* should control a value in reentrant events * should control values in reentrant events with different targets * should display `defaultValue` of number 0 * only assigns defaultValue if it changes @@ -898,6 +899,7 @@ src/renderers/dom/stack/client/__tests__/ReactDOM-test.js * should overwrite props.children with children argument * should purge the DOM cache when removing nodes * allow React.DOM factories to be called without warnings +* preserves focus src/renderers/dom/stack/client/__tests__/ReactMount-test.js * should render different components in same root @@ -1023,6 +1025,7 @@ src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js * can schedule updates after uncaught error during umounting * continues work on other roots despite caught errors * continues work on other roots despite uncaught errors +* restores environment state despite error during commit src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js * handles isMounted even when the initial render is deferred diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index 4cc6c2ac913e1..012a9d1335921 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -16,12 +16,14 @@ import type { Fiber } from 'ReactFiber'; import type { HostChildren } from 'ReactFiberReconciler'; import type { ReactNodeList } from 'ReactTypes'; +var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter'); var ReactControlledComponent = require('ReactControlledComponent'); var ReactDOMComponentTree = require('ReactDOMComponentTree'); var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags'); var ReactDOMFiberComponent = require('ReactDOMFiberComponent'); var ReactDOMInjection = require('ReactDOMInjection'); var ReactFiberReconciler = require('ReactFiberReconciler'); +var ReactInputSelection = require('ReactInputSelection'); var ReactInstanceMap = require('ReactInstanceMap'); var ReactPortal = require('ReactPortal'); @@ -29,11 +31,6 @@ var findDOMNode = require('findDOMNode'); var invariant = require('invariant'); var warning = require('warning'); -ReactDOMInjection.inject(); -ReactControlledComponent.injection.injectFiberControlledHostComponent( - ReactDOMFiberComponent -); - var { createElement, setInitialProperties, @@ -73,8 +70,22 @@ function recursivelyAppendChildren(parent : Element, child : HostChildren) : void { // TODO: Containers should update similarly to other parents. container.innerHTML = ''; diff --git a/src/renderers/dom/stack/client/__tests__/ReactDOM-test.js b/src/renderers/dom/stack/client/__tests__/ReactDOM-test.js index 7041673d927a7..f1a6575c12275 100644 --- a/src/renderers/dom/stack/client/__tests__/ReactDOM-test.js +++ b/src/renderers/dom/stack/client/__tests__/ReactDOM-test.js @@ -177,4 +177,64 @@ describe('ReactDOM', () => { 'to be a function. Instead received: Foo (keys: a, b).' ); }); + + it('preserves focus', () => { + let input; + let input2; + class A extends React.Component { + render() { + return ( +
+ input = input || r} /> + {this.props.showTwo && + input2 = input2 || r} />} +
+ ); + } + + componentDidUpdate() { + // Focus should have been restored to the original input + expect(document.activeElement.id).toBe('one'); + input2.focus(); + console.log('focus input2'); + expect(document.activeElement.id).toBe('two'); + log.push('input2 focused'); + } + } + + var log = []; + var container = document.createElement('div'); + document.body.appendChild(container); + ReactDOM.render(, container); + input.focus(); + + // When the second input is added, let's simulate losing focus, which is + // something that could happen when manipulating DOM nodes (but is hard to + // deterministically force without relying intensely on React DOM + // implementation details) + var div = container.firstChild; + ['appendChild', 'insertBefore'].forEach((name) => { + var mutator = div[name]; + div[name] = function() { + if (input) { + input.blur(); + expect(document.activeElement.tagName).toBe('BODY'); + log.push('input2 inserted'); + } + return mutator.apply(this, arguments); + }; + }); + + expect(document.activeElement.id).toBe('one'); + ReactDOM.render(, container); + // input2 gets added, which causes input to get blurred. Then + // componentDidUpdate focuses input2 and that should make it down to here, + // not get overwritten by focus restoration. + expect(document.activeElement.id).toBe('two'); + expect(log).toEqual([ + 'input2 inserted', + 'input2 focused', + ]); + document.body.removeChild(container); + }); }); diff --git a/src/renderers/noop/ReactNoop.js b/src/renderers/noop/ReactNoop.js index ebac23f6a24f9..6b446527a1c45 100644 --- a/src/renderers/noop/ReactNoop.js +++ b/src/renderers/noop/ReactNoop.js @@ -156,8 +156,23 @@ var NoopRenderer = ReactFiberReconciler({ scheduledDeferredCallback = callback; }, + prepareForCommit() : void { + if (isCommitting) { + throw new Error('Double prepare before commit'); + } + isCommitting = true; + }, + + resetAfterCommit() : void { + if (!isCommitting) { + throw new Error('Double reset after commit'); + } + isCommitting = false; + }, + }); +var isCommitting = false; var rootContainers = new Map(); var roots = new Map(); var DEFAULT_ROOT_ID = ''; @@ -255,6 +270,8 @@ var ReactNoop = { syncUpdates: NoopRenderer.syncUpdates, + isCommitting: () => isCommitting, + // Logs the current state of the tree. dumpTree(rootID : string = DEFAULT_ROOT_ID) { const root = roots.get(rootID); diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index b1344c412137e..d782fd024d088 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -66,6 +66,9 @@ export type HostConfig = { scheduleAnimationCallback(callback : () => void) : void, scheduleDeferredCallback(callback : (deadline : Deadline) => void) : void, + prepareForCommit() : void, + resetAfterCommit() : void, + useSyncScheduling ?: boolean, }; diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 71aff51a26bec..b72a70a4d0930 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -74,6 +74,9 @@ module.exports = function(config : HostConfig) { const hostScheduleDeferredCallback = config.scheduleDeferredCallback; const useSyncScheduling = config.useSyncScheduling; + const prepareForCommit = config.prepareForCommit; + const resetAfterCommit = config.resetAfterCommit; + // The priority level to use when scheduling an update. let priorityContext : PriorityLevel = useSyncScheduling ? SynchronousPriority : @@ -85,6 +88,10 @@ module.exports = function(config : HostConfig) { // Need this to prevent recursion while in a Task loop. let isPerformingTaskWork : boolean = false; + // We'll only prepare/reset on the outermost commit even when a setState + // callback causes another synchronous rerender + let isCommitting : boolean = false; + // The next work in progress fiber that we're currently working on. let nextUnitOfWork : ?Fiber = null; let nextPriorityLevel : PriorityLevel = NoWork; @@ -161,45 +168,66 @@ module.exports = function(config : HostConfig) { } function commitAllWork(finishedWork : Fiber) { - // Commit all the side-effects within a tree. - // First, we'll perform all the host insertions, updates, deletions and - // ref unmounts. - let effectfulFiber = finishedWork.firstEffect; - while (effectfulFiber) { - switch (effectfulFiber.effectTag) { - case Placement: - case PlacementAndCallback: { - commitInsertion(effectfulFiber); - // Clear the "placement" from effect tag so that we know that this is inserted, before - // any life-cycles like componentDidMount gets called. - effectfulFiber.effectTag ^= Placement; - break; - } - case PlacementAndUpdate: - case PlacementAndUpdateAndCallback: { - // Placement - commitInsertion(effectfulFiber); - // Clear the "placement" from effect tag so that we know that this is inserted, before - // any life-cycles like componentDidMount gets called. - effectfulFiber.effectTag ^= Placement; - - // Update - const current = effectfulFiber.alternate; - commitWork(current, effectfulFiber); - break; + let effectfulFiber; + + if (!isCommitting) { + isCommitting = true; + prepareForCommit(); + } + + try { + // Commit all the side-effects within a tree. + // First, we'll perform all the host insertions, updates, deletions and + // ref unmounts. + effectfulFiber = finishedWork.firstEffect; + while (effectfulFiber) { + switch (effectfulFiber.effectTag) { + case Placement: + case PlacementAndCallback: { + commitInsertion(effectfulFiber); + // Clear the "placement" from effect tag so that we know that this is inserted, before + // any life-cycles like componentDidMount gets called. + effectfulFiber.effectTag ^= Placement; + break; + } + case PlacementAndUpdate: + case PlacementAndUpdateAndCallback: { + // Placement + commitInsertion(effectfulFiber); + // Clear the "placement" from effect tag so that we know that this is inserted, before + // any life-cycles like componentDidMount gets called. + effectfulFiber.effectTag ^= Placement; + + // Update + const current = effectfulFiber.alternate; + commitWork(current, effectfulFiber); + break; + } + case Update: + case UpdateAndCallback: + const current = effectfulFiber.alternate; + commitWork(current, effectfulFiber); + break; + case Deletion: + case DeletionAndCallback: + commitDeletion(effectfulFiber); + break; } - case Update: - case UpdateAndCallback: - const current = effectfulFiber.alternate; - commitWork(current, effectfulFiber); - break; - case Deletion: - case DeletionAndCallback: - commitDeletion(effectfulFiber); - break; + + effectfulFiber = effectfulFiber.nextEffect; } - effectfulFiber = effectfulFiber.nextEffect; + // Finally if the root itself had an effect, we perform that since it is + // not part of the effect list. + if (finishedWork.effectTag !== NoEffect) { + const current = finishedWork.alternate; + commitWork(current, finishedWork); + } + } finally { + if (isCommitting) { + isCommitting = false; + resetAfterCommit(); + } } // Next, we'll perform all life-cycles and ref callbacks. Life-cycles @@ -228,11 +256,9 @@ module.exports = function(config : HostConfig) { effectfulFiber = next; } - // Finally if the root itself had an effect, we perform that since it is not - // part of the effect list. + // Lifecycles on the root itself if (finishedWork.effectTag !== NoEffect) { const current = finishedWork.alternate; - commitWork(current, finishedWork); commitLifeCycles(current, finishedWork); } diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js index 0f55886650227..e906780b8f890 100644 --- a/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js +++ b/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js @@ -275,4 +275,30 @@ describe('ReactIncrementalErrorHandling', () => { expect(ReactNoop.getChildren('e')).toEqual(null); expect(ReactNoop.getChildren('f')).toEqual(null); }); + + it('restores environment state despite error during commit', () => { + class Text extends React.Component { + render() { + return ; + } + } + + let text; + ReactNoop.render( text = t}>alpha); + ReactNoop.flush(); + let textInstance = ReactNoop.findInstance(text); + expect(textInstance.prop).toBe('alpha'); + + // Make the commit phase throw an error + Object.defineProperty(textInstance, 'prop', { + set: function() { + expect(ReactNoop.isCommitting()).toBe(true); + throw new Error('Failed to commit'); + }, + }); + + ReactNoop.render(beta); + expect(ReactNoop.flush).toThrow('Failed to commit'); + expect(ReactNoop.isCommitting()).toBe(false); + }); });