Skip to content

Commit e9615b7

Browse files
committed
Make disableNewFiberFeatures = true the default when runnings tests
This exposed a few more cases that I missed.
1 parent fa2f568 commit e9615b7

17 files changed

+143
-87
lines changed

scripts/fiber/tests-passing.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -549,7 +549,6 @@ src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
549549
* should not crash encountering low-priority tree
550550
* throws if non-element passed to top-level render
551551
* throws if something other than false, null, or an element is returned from render
552-
* still accepts arrays and iterables as host children
553552

554553
src/renderers/dom/shared/__tests__/CSSProperty-test.js
555554
* should generate browser prefixes for its `isUnitlessNumber`

scripts/jest/test-framework-setup.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@ jest.mock('ReactDOMFeatureFlags', () => {
99
useFiber: flags.useFiber || !!process.env.REACT_DOM_JEST_USE_FIBER,
1010
});
1111
});
12+
jest.mock('ReactFeatureFlags', () => {
13+
const flags = require.requireActual('ReactFeatureFlags');
14+
return Object.assign({}, flags, {
15+
disableNewFiberFeatures: true,
16+
});
17+
});
1218

1319
// Error logging varies between Fiber and Stack;
1420
// Rather than fork dozens of tests, mock the error-logging file by default.

src/renderers/dom/fiber/ReactDOMFiber.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import type { ReactNodeList } from 'ReactTypes';
1818
var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter');
1919
var ReactControlledComponent = require('ReactControlledComponent');
2020
var ReactDOMComponentTree = require('ReactDOMComponentTree');
21+
var ReactFeatureFlags = require('ReactFeatureFlags');
2122
var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
2223
var ReactDOMFiberComponent = require('ReactDOMFiberComponent');
2324
var ReactDOMInjection = require('ReactDOMInjection');
@@ -26,6 +27,8 @@ var ReactFiberReconciler = require('ReactFiberReconciler');
2627
var ReactInputSelection = require('ReactInputSelection');
2728
var ReactInstanceMap = require('ReactInstanceMap');
2829
var ReactPortal = require('ReactPortal');
30+
var { isValidElement } = require('ReactElement');
31+
2932

3033
var findDOMNode = require('findDOMNode');
3134
var invariant = require('invariant');
@@ -44,6 +47,7 @@ if (__DEV__) {
4447
var { updatedAncestorInfo } = validateDOMNesting;
4548
}
4649

50+
4751
const DOCUMENT_NODE = 9;
4852

4953
ReactDOMInjection.inject();
@@ -345,6 +349,17 @@ var ReactDOM = {
345349

346350
render(element : ReactElement<any>, container : DOMContainerElement, callback: ?Function) {
347351
validateContainer(container);
352+
353+
if (ReactFeatureFlags.disableNewFiberFeatures) {
354+
// Top-level check occurs here instead of inside child reconciler because
355+
// because requirements vary between renderers. E.g. React Art
356+
// allows arrays.
357+
invariant(
358+
isValidElement(element),
359+
'render(): Invalid component element.'
360+
);
361+
}
362+
348363
return renderSubtreeIntoContainer(null, element, container, callback);
349364
},
350365

src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js

Lines changed: 40 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,17 @@ var ReactTestUtils = require('ReactTestUtils');
1818

1919
describe('ReactDOMFiber', () => {
2020
var container;
21+
var ReactFeatureFlags;
2122

2223
beforeEach(() => {
2324
container = document.createElement('div');
25+
ReactFeatureFlags = require('ReactFeatureFlags');
26+
ReactFeatureFlags.disableNewFiberFeatures = false;
27+
});
28+
29+
afterEach(() => {
30+
ReactFeatureFlags = require('ReactFeatureFlags');
31+
ReactFeatureFlags.disableNewFiberFeatures = true;
2432
});
2533

2634
it('should render strings as children', () => {
@@ -1002,47 +1010,42 @@ describe('ReactDOMFiber', () => {
10021010
container
10031011
);
10041012
});
1013+
}
1014+
});
10051015

1006-
describe('disableNewFiberFeatures', () => {
1007-
var ReactFeatureFlags = require('ReactFeatureFlags');
1008-
1009-
beforeEach(() => {
1010-
ReactFeatureFlags.disableNewFiberFeatures = true;
1011-
});
1016+
// disableNewFiberFeatures currently defaults to true in test
1017+
describe('disableNewFiberFeatures', () => {
1018+
var container;
1019+
var ReactFeatureFlags;
10121020

1013-
afterEach(() => {
1014-
ReactFeatureFlags.disableNewFiberFeatures = false;
1015-
});
1021+
beforeEach(() => {
1022+
container = document.createElement('div');
1023+
ReactFeatureFlags = require('ReactFeatureFlags');
1024+
ReactFeatureFlags.disableNewFiberFeatures = true;
1025+
});
10161026

1017-
it('throws if non-element passed to top-level render', () => {
1018-
const message = 'render(): Invalid component element.';
1019-
expect(() => ReactDOM.render(null, container)).toThrow(message, container);
1020-
expect(() => ReactDOM.render(undefined, container)).toThrow(message, container);
1021-
expect(() => ReactDOM.render(false, container)).toThrow(message, container);
1022-
expect(() => ReactDOM.render('Hi', container)).toThrow(message, container);
1023-
expect(() => ReactDOM.render(999, container)).toThrow(message, container);
1024-
expect(() => ReactDOM.render([<div />], container)).toThrow(message, container);
1025-
});
1027+
afterEach(() => {
1028+
ReactFeatureFlags = require('ReactFeatureFlags');
1029+
ReactFeatureFlags.disableNewFiberFeatures = false;
1030+
});
10261031

1027-
it('throws if something other than false, null, or an element is returned from render', () => {
1028-
function Render(props) {
1029-
return props.children;
1030-
}
1032+
it('throws if non-element passed to top-level render', () => {
1033+
const message = 'render(): Invalid component element.';
1034+
expect(() => ReactDOM.render(null, container)).toThrow(message, container);
1035+
expect(() => ReactDOM.render(undefined, container)).toThrow(message, container);
1036+
expect(() => ReactDOM.render(false, container)).toThrow(message, container);
1037+
expect(() => ReactDOM.render('Hi', container)).toThrow(message, container);
1038+
expect(() => ReactDOM.render(999, container)).toThrow(message, container);
1039+
expect(() => ReactDOM.render([<div />], container)).toThrow(message, container);
1040+
});
10311041

1032-
expect(() => ReactDOM.render(<Render>Hi</Render>, container)).toThrow(/You may have returned undefined/);
1033-
expect(() => ReactDOM.render(<Render>{999}</Render>, container)).toThrow(/You may have returned undefined/);
1034-
expect(() => ReactDOM.render(<Render>[<div />]</Render>, container)).toThrow(/You may have returned undefined/);
1035-
});
1042+
it('throws if something other than false, null, or an element is returned from render', () => {
1043+
function Render(props) {
1044+
return props.children;
1045+
}
10361046

1037-
it('still accepts arrays and iterables as host children', () => {
1038-
function Render(props) {
1039-
return <div>{props.children}</div>;
1040-
}
1041-
ReactDOM.render(<Render>{['foo', 'bar']}</Render>, container);
1042-
expect(container.textContent).toEqual('foobar');
1043-
ReactDOM.render(<Render>{new Set(['foo', 'bar'])}</Render>, container);
1044-
expect(container.textContent).toEqual('foobar');
1045-
});
1046-
});
1047-
}
1047+
expect(() => ReactDOM.render(<Render>Hi</Render>, container)).toThrow(/You may have returned undefined/);
1048+
expect(() => ReactDOM.render(<Render>{999}</Render>, container)).toThrow(/You may have returned undefined/);
1049+
expect(() => ReactDOM.render(<Render>[<div />]</Render>, container)).toThrow(/You may have returned undefined/);
1050+
});
10481051
});

src/renderers/shared/fiber/ReactChildFiber.js

Lines changed: 31 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ const {
7575
NoEffect,
7676
Placement,
7777
Deletion,
78-
Err,
7978
} = ReactTypeOfSideEffect;
8079

8180
function coerceRef(current: ?Fiber, element: ReactElement) {
@@ -1127,41 +1126,40 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
11271126
}
11281127
}
11291128

1130-
if (returnFiber.tag === HostRoot) {
1131-
// Top-level only accepts elements or portals
1132-
invariant(
1133-
// If the root has an error effect, this is an intentional unmount.
1134-
// Don't throw an error.
1135-
returnFiber.effectTag & Err,
1136-
'render(): Invalid component element.'
1137-
);
1138-
} else {
1139-
switch (returnFiber.tag) {
1140-
case ClassComponent: {
1141-
if (__DEV__) {
1142-
const instance = returnFiber.stateNode;
1143-
if (instance.render._isMockFunction) {
1144-
// We allow auto-mocks to proceed as if they're
1145-
// returning null.
1146-
break;
1147-
}
1129+
switch (returnFiber.tag) {
1130+
case ClassComponent: {
1131+
if (__DEV__) {
1132+
const instance = returnFiber.stateNode;
1133+
if (instance.render._isMockFunction) {
1134+
// We allow auto-mocks to proceed as if they're
1135+
// returning null.
1136+
break;
11481137
}
11491138
}
1150-
// Intentionally fall through to the next case, which handles both
1151-
// functions and classes
1152-
// eslint-disable-next-lined no-fallthrough
1153-
case FunctionalComponent: {
1154-
// Composites accept elements, portals, null, or false
1155-
const Component = returnFiber.type;
1156-
invariant(
1157-
newChild === null || newChild === false,
1158-
'%s.render(): A valid React element (or null) must be ' +
1159-
'returned. You may have returned undefined, an array or some ' +
1160-
'other invalid object.',
1161-
Component.displayName || Component.name || 'Component'
1162-
);
1163-
}
11641139
}
1140+
// Intentionally fall through to the next case, which handles both
1141+
// functions and classes
1142+
// eslint-disable-next-lined no-fallthrough
1143+
case FunctionalComponent: {
1144+
// Composites accept elements, portals, null, or false
1145+
const Component = returnFiber.type;
1146+
invariant(
1147+
newChild === null || newChild === false,
1148+
'%s(...): A valid React element (or null) must be ' +
1149+
'returned. You may have returned undefined, an array or some ' +
1150+
'other invalid object.',
1151+
Component.displayName || Component.name || 'Component'
1152+
);
1153+
}
1154+
}
1155+
1156+
if (typeof newChild === 'string' || typeof newChild === 'number') {
1157+
return placeSingleChild(reconcileSingleTextNode(
1158+
returnFiber,
1159+
currentFirstChild,
1160+
'' + newChild,
1161+
priority
1162+
));
11651163
}
11661164

11671165
if (isArray(newChild)) {

src/renderers/shared/fiber/__tests__/ReactCoroutine-test.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,16 @@
1414
var React;
1515
var ReactNoop;
1616
var ReactCoroutine;
17+
var ReactFeatureFlags;
1718

1819
describe('ReactCoroutine', () => {
1920
beforeEach(() => {
2021
jest.resetModules();
2122
React = require('React');
2223
ReactNoop = require('ReactNoop');
2324
ReactCoroutine = require('ReactCoroutine');
25+
ReactFeatureFlags = require('ReactFeatureFlags');
26+
ReactFeatureFlags.disableNewFiberFeatures = false;
2427
});
2528

2629
it('should render a coroutine', () => {

src/renderers/shared/fiber/__tests__/ReactIncremental-test.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,16 @@
1313

1414
var React;
1515
var ReactNoop;
16+
var ReactFeatureFlags;
1617

1718
describe('ReactIncremental', () => {
1819
beforeEach(() => {
1920
jest.resetModules();
2021
React = require('React');
2122
ReactNoop = require('ReactNoop');
23+
24+
ReactFeatureFlags = require('ReactFeatureFlags');
25+
ReactFeatureFlags.disableNewFiberFeatures = false;
2226
});
2327

2428
it('should render a simple component', () => {

src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,15 @@
1313

1414
var React;
1515
var ReactNoop;
16+
var ReactFeatureFlags;
1617

1718
describe('ReactIncrementalErrorHandling', () => {
1819
beforeEach(() => {
1920
jest.resetModules();
2021
React = require('React');
2122
ReactNoop = require('ReactNoop');
23+
ReactFeatureFlags = require('ReactFeatureFlags');
24+
ReactFeatureFlags.disableNewFiberFeatures = false;
2225
});
2326

2427
function div(...children) {

src/renderers/shared/fiber/__tests__/ReactIncrementalReflection-test.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,15 @@
1313

1414
var React;
1515
var ReactNoop;
16+
var ReactFeatureFlags;
1617

1718
describe('ReactIncrementalReflection', () => {
1819
beforeEach(() => {
1920
jest.resetModules();
2021
React = require('React');
2122
ReactNoop = require('ReactNoop');
23+
ReactFeatureFlags = require('ReactFeatureFlags');
24+
ReactFeatureFlags.disableNewFiberFeatures = false;
2225
});
2326

2427
it('handles isMounted even when the initial render is deferred', () => {

src/renderers/shared/fiber/__tests__/ReactIncrementalScheduling-test.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,15 @@
1313

1414
var React;
1515
var ReactNoop;
16+
var ReactFeatureFlags;
1617

1718
describe('ReactIncrementalScheduling', () => {
1819
beforeEach(() => {
1920
jest.resetModules();
2021
React = require('React');
2122
ReactNoop = require('ReactNoop');
23+
ReactFeatureFlags = require('ReactFeatureFlags');
24+
ReactFeatureFlags.disableNewFiberFeatures = false;
2225
});
2326

2427
function span(prop) {

0 commit comments

Comments
 (0)