@samitouri / QOS-React / commits / 6c26e98ec2

Handle case where host environment throws when working on failed root

Otherwise you fall into an infinite loop where root fails -> host env throws -> root fails... This is a worst-case scenario that should only be possible if there's a bug in the renderer.

Andrew Clark committed Dec 19, 2016 at 18:06 UTC 6c26e98ec2e61476f66edaa3ef46a283a9d0e8f8

6 files changed +54 -12

scripts/fiber/tests-failing.txt

-1

@@ -64,7 +64,6 @@ src/renderers/shared/hooks/__tests__/ReactHostOperationHistoryHook-test.js

64 * gets recorded during an update

66 src/renderers/shared/shared/__tests__/ReactComponent-test.js

67 -* should throw on invalid render targets

67 * includes owner name in the error about badly-typed elements

69 src/renderers/shared/shared/__tests__/ReactComponentLifeCycle-test.js

scripts/fiber/tests-passing.txt

@@ -1319,6 +1319,7 @@ src/renderers/shared/shared/__tests__/ReactChildReconciler-test.js

1319 * warns for duplicated iterable keys with component stack info

1320

1321 src/renderers/shared/shared/__tests__/ReactComponent-test.js

1322 +* should throw on invalid render targets

1323 * should throw when supplying a ref outside of render method

1324 * should warn when children are mutated during render

1325 * should warn when children are mutated during update

src/renderers/dom/fiber/ReactDOMFiber.js

+33 -4

@@ -71,6 +71,31 @@ type HostContext = HostContextDev | HostContextProd;

71 let eventsEnabled : ?boolean = null;

72 let selectionInformation : ?mixed = null;

74 +var ELEMENT_NODE_TYPE = 1;

75 +var DOC_NODE_TYPE = 9;

76 +var DOCUMENT_FRAGMENT_NODE_TYPE = 11;

77 +

78 +/**

79 + * True if the supplied DOM node is a valid node element.

80 + *

81 + * @param {?DOMElement} node The candidate DOM node.

82 + * @return {boolean} True if the DOM is a valid DOM node.

83 + * @internal

84 + */

85 +function isValidContainer(node) {

86 + return !!(node && (

87 + node.nodeType === ELEMENT_NODE_TYPE ||

88 + node.nodeType === DOC_NODE_TYPE ||

89 + node.nodeType === DOCUMENT_FRAGMENT_NODE_TYPE

90 + ));

91 +}

92 +

93 +function validateContainer(container) {

94 + if (!isValidContainer(container)) {

95 + throw new Error('Target container is not a DOM element.');

96 + }

97 +}

98 +

99 var DOMRenderer = ReactFiberReconciler({

100

101 getRootHostContext(rootContainerInstance : Container) : HostContext {

@@ -260,6 +285,8 @@ function warnAboutUnstableUse() {

285 }

286

287 function renderSubtreeIntoContainer(parentComponent : ?ReactComponent<any, any, any>, children : ReactNodeList, containerNode : DOMContainerElement | Document, callback: ?Function) {

288 + validateContainer(containerNode);

289 +

290 let container : DOMContainerElement =

291 containerNode.nodeType === DOCUMENT_NODE ? (containerNode : any).documentElement : (containerNode : any);

292 let root;

@@ -278,7 +305,7 @@ function renderSubtreeIntoContainer(parentComponent : ?ReactComponent<any, any,

305 var ReactDOM = {

306

307 render(element : ReactElement<any>, container : DOMContainerElement, callback: ?Function) {

281 - warnAboutUnstableUse();

308 + validateContainer(container);

309 return renderSubtreeIntoContainer(null, element, container, callback);

310 },

311

@@ -292,9 +319,11 @@ var ReactDOM = {

319

320 unmountComponentAtNode(container : DOMContainerElement) {

321 warnAboutUnstableUse();

295 - return renderSubtreeIntoContainer(null, null, container, () => {

296 - container._reactRootContainer = null;

297 - });

322 + if (container._reactRootContainer) {

323 + return renderSubtreeIntoContainer(null, null, container, () => {

324 + container._reactRootContainer = null;

325 + });

326 + }

327 },

328

329 findDOMNode: findDOMNode,

src/renderers/shared/fiber/ReactFiberReconciler.js

+2 -3

@@ -143,10 +143,9 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C

143 if (__DEV__) {

144 if (ReactFiberInstrumentation.debugTool) {

145 if (element === null) {

146 - ReactFiberInstrumentation.debugTool.onUpdateContainer(root);

147 - } else {

148 - // This is an unmount

146 ReactFiberInstrumentation.debugTool.onUnmountContainer(root);

147 + } else {

148 + ReactFiberInstrumentation.debugTool.onUpdateContainer(root);

149 }

150 }

151 }

src/renderers/shared/fiber/ReactFiberScheduler.js

+16 -2

@@ -141,6 +141,7 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C

141 // Error boundaries that captured an error during the current commit.

142 let commitPhaseBoundaries : Set<Fiber> | null = null;

143 let firstUncaughtError : Error | null = null;

144 + let fatalError : Error | null = null;

145

146 let isCommitting : boolean = false;

147 let isUnmounting : boolean = false;

@@ -670,7 +671,7 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C

671 // This outer loop exists so that we can restart the work loop after

672 // catching an error. It also lets us flush Task work at the end of a

673 // deferred batch.

673 - while (priorityLevel !== NoWork) {

674 + while (priorityLevel !== NoWork && !fatalError) {

675 if (priorityLevel >= HighPriority && !deadline) {

676 throw new Error(

677 'Cannot perform deferred work without a deadline.'

@@ -769,7 +770,13 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C

770 capturedErrors = null;

771 failedBoundaries = null;

772

772 - // It's now safe to throw the first uncaught error.

773 + // It's now safe to throw errors.

774 + if (fatalError) {

775 + let e = fatalError;

776 + fatalError = null;

777 + firstUncaughtError = null;

778 + throw e;

779 + }

780 if (firstUncaughtError) {

781 let e = firstUncaughtError;

782 firstUncaughtError = null;

@@ -795,6 +802,13 @@ module.exports = function<T, P, I, TI, C, CX>(config : HostConfig<T, P, I, TI, C

802 // ignore the work itself and only search through the parents.

803 if (failedWork.tag === HostRoot) {

804 boundary = failedWork;

805 +

806 + if (isFailedBoundary(failedWork)) {

807 + // If this root already failed, there must have been an error when

808 + // attempting to unmount it. This is a worst-case scenario and

809 + // should only be possible if there's a bug in the renderer.

810 + fatalError = error;

811 + }

812 } else {

813 let node = failedWork.return;

814 while (node && !boundary) {

src/renderers/shared/shared/__tests__/ReactComponent-test.js

+2 -2

@@ -30,13 +30,13 @@ describe('ReactComponent', () => {

30 expect(function() {

31 ReactDOM.render(<div />, [container]);

32 }).toThrowError(

33 - '_registerComponent(...): Target container is not a DOM element.'

33 + /Target container is not a DOM element./

34 );

36 expect(function() {

37 ReactDOM.render(<div />, null);

38 }).toThrowError(

39 - '_registerComponent(...): Target container is not a DOM element.'

39 + /Target container is not a DOM element./

40 );

41 });