Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions scripts/fiber/tests-failing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,6 @@ src/renderers/shared/shared/__tests__/ReactEmptyComponent-test.js
src/renderers/shared/shared/__tests__/ReactMultiChildText-test.js
* should reorder keyed text nodes

src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js
* should warn when stateless component returns array
* should warn when using non-React functions in JSX

src/renderers/shared/shared/__tests__/ReactUpdates-test.js
* marks top-level updates

Expand Down
3 changes: 3 additions & 0 deletions scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,8 @@ src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
* should bubble events from the portal to the parent
* should not onMouseLeave when staying in the portal
* should not crash encountering low-priority tree
* throws if non-element passed to top-level render
* throws if something other than false, null, or an element is returned from render

src/renderers/dom/shared/__tests__/CSSProperty-test.js
* should generate browser prefixes for its `isUnitlessNumber`
Expand Down Expand Up @@ -1536,6 +1538,7 @@ src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js
* should update stateless component
* should unmount stateless component
* should pass context thru stateless component
* should throw when stateless component returns undefined
* should throw on string refs in pure functions
* should warn when given a string ref
* should warn when given a function ref
Expand Down
35 changes: 35 additions & 0 deletions src/renderers/dom/fiber/__tests__/ReactDOMFiber-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1002,5 +1002,40 @@ describe('ReactDOMFiber', () => {
container
);
});

describe('disableNewFiberFeatures', () => {
var ReactFeatureFlags = require('ReactFeatureFlags');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could change var to const

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're already pretty inconsistent throughout the codebase :D One day we'll go through and clean this up


beforeEach(() => {
ReactFeatureFlags.disableNewFiberFeatures = true;
});

afterEach(() => {
ReactFeatureFlags.disableNewFiberFeatures = false;
});

it('throws if non-element passed to top-level render', () => {
// FIXME: These assertions pass individually, but they leave React in
// an inconsistent state. This suggests an error-handling bug. I'll fix
// this in a separate PR.
const message = 'render(): Invalid component element.';
expect(() => ReactDOM.render(null, container)).toThrow(message, container);
expect(() => ReactDOM.render(undefined, container)).toThrow(message, container);
expect(() => ReactDOM.render(false, container)).toThrow(message, container);
expect(() => ReactDOM.render('Hi', container)).toThrow(message, container);
expect(() => ReactDOM.render(999, container)).toThrow(message, container);
expect(() => ReactDOM.render([<div />], container)).toThrow(message, container);
});

it('throws if something other than false, null, or an element is returned from render', () => {
function Render(props) {
return props.children;
}

expect(() => ReactDOM.render(<Render>Hi</Render>, container)).toThrow(/You may have returned undefined/);
expect(() => ReactDOM.render(<Render>{999}</Render>, container)).toThrow(/You may have returned undefined/);
expect(() => ReactDOM.render(<Render>[<div />]</Render>, container)).toThrow(/You may have returned undefined/);
});
});
}
});
96 changes: 96 additions & 0 deletions src/renderers/shared/fiber/ReactChildFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ var ReactTypeOfWork = require('ReactTypeOfWork');
var emptyObject = require('emptyObject');
var getIteratorFn = require('getIteratorFn');
var invariant = require('invariant');
var ReactFeatureFlags = require('ReactFeatureFlags');

if (__DEV__) {
var { getCurrentFiberStackAddendum } = require('ReactDebugCurrentFiber');
Expand All @@ -60,8 +61,10 @@ const {
const isArray = Array.isArray;

const {
FunctionalComponent,
ClassComponent,
HostText,
HostRoot,
HostPortal,
CoroutineComponent,
YieldComponent,
Expand All @@ -72,6 +75,7 @@ const {
NoEffect,
Placement,
Deletion,
Err,
} = ReactTypeOfSideEffect;

function coerceRef(current: ?Fiber, element: ReactElement) {
Expand Down Expand Up @@ -1100,6 +1104,68 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
// not as a fragment. Nested arrays on the other hand will be treated as
// fragment nodes. Recursion happens at the normal flow.

if (ReactFeatureFlags.disableNewFiberFeatures) {
// Support only the subset of return types that Stack supports. Treat
// everything else as empty, but log a warning.
if (typeof newChild === 'object' && newChild !== null) {
switch (newChild.$$typeof) {
case REACT_ELEMENT_TYPE:
return placeSingleChild(reconcileSingleElement(
returnFiber,
currentFirstChild,
newChild,
priority
));

case REACT_PORTAL_TYPE:
return placeSingleChild(reconcileSinglePortal(
returnFiber,
currentFirstChild,
newChild,
priority
));
}
}

if (returnFiber.tag === HostRoot) {
// Top-level only accepts elements or portals
invariant(
// If the root has an error effect, this is an intentional unmount.
// Don't throw an error.
returnFiber.effectTag & Err,
'render(): Invalid component element.'
);
} else {
switch (returnFiber.tag) {
case ClassComponent: {
if (__DEV__) {
const instance = returnFiber.stateNode;
if (instance.render._isMockFunction) {
// We allow auto-mocks to proceed as if they're
// returning null.
break;
}
}
}
// Intentionally fall through to the next case, which handles both
// functions and classes
// eslint-disable-next-lined no-fallthrough
case FunctionalComponent: {
// Composites accept elements, portals, null, or false
const Component = returnFiber.type;
invariant(
newChild === null || newChild === false,
'%s.render(): A valid React element (or null) must be ' +
'returned. You may have returned undefined, an array or some ' +
'other invalid object.',
Component.displayName || Component.name || 'Component'
);
}
}
}
return deleteRemainingChildren(returnFiber, currentFirstChild);
}

if (typeof newChild === 'string' || typeof newChild === 'number') {
return placeSingleChild(reconcileSingleTextNode(
returnFiber,
Expand Down Expand Up @@ -1163,6 +1229,36 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
}
}

if (typeof newChild === 'undefined') {
switch (returnFiber.tag) {
case HostRoot:
// TODO: Top-level render
break;
case ClassComponent: {
if (__DEV__) {
const instance = returnFiber.stateNode;
if (instance.render._isMockFunction) {
// We allow auto-mocks to proceed as if they're returning null.
break;
}
}
}
// Intentionally fall through to the next case, which handles both
// functions and classes
// eslint-disable-next-lined no-fallthrough
case FunctionalComponent: {
const Component = returnFiber.type;
invariant(
false,
'%s(...): Nothing was returned from render. This usually means a ' +
'return statement is missing. Or, to render nothing, ' +
'return null.',
Component.displayName || Component.name || 'Component'
);
}
}
}

// Remaining cases are all treated as empty.
return deleteRemainingChildren(returnFiber, currentFirstChild);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -956,6 +956,7 @@ describe('ReactIncremental', () => {

function Trail() {
ops.push('Trail');
return null;
}

function App(props) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,7 @@ describe('ReactIncrementalErrorHandling', () => {
if (props.throw) {
throw new Error('Hello');
}
return null;
}

function Foo() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ var React;
var ReactDOM;
var ReactTestUtils;

var ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');

function StatelessComponent(props) {
return <div>{props.name}</div>;
}
Expand Down Expand Up @@ -123,18 +125,34 @@ describe('ReactStatelessComponent', () => {
);
});

it('should warn when stateless component returns array', () => {
spyOn(console, 'error');
if (!ReactDOMFeatureFlags.useFiber) {
// Stack doesn't support fragments
it('should throw when stateless component returns array', () => {
function NotAComponent() {
return [<div />, <div />];
}
expect(function() {
ReactTestUtils.renderIntoDocument(<div><NotAComponent /></div>);
}).toThrowError(
'NotAComponent(...): A valid React element (or null) must be returned. ' +
'You may have returned undefined, an array or some other invalid object.'
);
});
}

it('should throw when stateless component returns undefined', () => {
function NotAComponent() {
return [<div />, <div />];
}
expect(function() {
ReactTestUtils.renderIntoDocument(<div><NotAComponent /></div>);
}).toThrow();
expectDev(console.error.calls.count()).toBe(1);
expectDev(console.error.calls.argsFor(0)[0]).toContain(
'NotAComponent(...): A valid React element (or null) must be returned. ' +
'You may have returned undefined, an array or some other invalid object.'
}).toThrowError(
ReactDOMFeatureFlags.useFiber ?
// Fiber gives a more specific error message for undefined because it
// supports more return types.
'NotAComponent(...): Nothing was returned from render' :
// Stack's message is generic.
'NotAComponent(...): A valid React element (or null) must be returned. ' +
'You may have returned undefined, an array or some other invalid object.'
);
});

Expand Down Expand Up @@ -374,18 +392,4 @@ describe('ReactStatelessComponent', () => {
expect(() => ReactTestUtils.renderIntoDocument(<Child />)).not.toThrow();
});

it('should warn when using non-React functions in JSX', () => {
spyOn(console, 'error');
function NotAComponent() {
return [<div />, <div />];
}
expect(function() {
ReactTestUtils.renderIntoDocument(<div><NotAComponent /></div>);
}).toThrow(); // has no method 'render'
expectDev(console.error.calls.count()).toBe(1);
expectDev(console.error.calls.argsFor(0)[0]).toContain(
'NotAComponent(...): A valid React element (or null) must be returned. You may ' +
'have returned undefined, an array or some other invalid object.'
);
});
});
25 changes: 7 additions & 18 deletions src/renderers/shared/stack/reconciler/ReactCompositeComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,26 +39,9 @@ function StatelessComponent(Component) {
StatelessComponent.prototype.render = function() {
var Component = ReactInstanceMap.get(this)._currentElement.type;
var element = Component(this.props, this.context, this.updater);
warnIfInvalidElement(Component, element);
return element;
};

function warnIfInvalidElement(Component, element) {
if (__DEV__) {
warning(
element === null || element === false || React.isValidElement(element),
'%s(...): A valid React element (or null) must be returned. You may have ' +
'returned undefined, an array or some other invalid object.',
Component.displayName || Component.name || 'Component'
);
warning(
!Component.childContextTypes,
'%s(...): childContextTypes cannot be defined on a functional component.',
Component.displayName || Component.name || 'Component'
);
}
}

function shouldConstruct(Component) {
return !!(Component.prototype && Component.prototype.isReactComponent);
}
Expand Down Expand Up @@ -205,7 +188,13 @@ var ReactCompositeComponent = {
// Support functional components
if (!doConstruct && (inst == null || inst.render == null)) {
renderedElement = inst;
warnIfInvalidElement(Component, renderedElement);
if (__DEV__) {
warning(
!Component.childContextTypes,
'%s(...): childContextTypes cannot be defined on a functional component.',
Component.displayName || Component.name || 'Component'
);
}
invariant(
inst === null ||
inst === false ||
Expand Down
1 change: 1 addition & 0 deletions src/renderers/shared/utils/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ var ReactFeatureFlags = {
// timeline profiles in Chrome, for example.
logTopLevelRenders: false,
prepareNewChildrenBeforeUnmountInStack: true,
disableNewFiberFeatures: false,
};

module.exports = ReactFeatureFlags;