Allow components to render undefined (#21869)

This commit is contained in:
Ricky 2021-07-13 15:48:11 -04:00 committed by GitHub
parent 87b67d319f
commit 14bac6193a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 313 additions and 443 deletions

View File

@ -924,33 +924,37 @@ describe('ReactDOMServerIntegration', () => {
);
});
describe('components that throw errors', function() {
itThrowsWhenRendering(
'a function returning undefined',
async render => {
const UndefinedComponent = () => undefined;
await render(<UndefinedComponent />, 1);
},
'UndefinedComponent(...): Nothing was returned from render. ' +
'This usually means a return statement is missing. Or, to ' +
'render nothing, return null.',
);
describe('components that render nullish', function() {
itRenders('a function returning null', async render => {
const NullComponent = () => null;
await render(<NullComponent />);
});
itThrowsWhenRendering(
'a class returning undefined',
async render => {
itRenders('a class returning null', async render => {
class NullComponent extends React.Component {
render() {
return null;
}
}
await render(<NullComponent />);
});
itRenders('a function returning undefined', async render => {
const UndefinedComponent = () => undefined;
await render(<UndefinedComponent />);
});
itRenders('a class returning undefined', async render => {
class UndefinedComponent extends React.Component {
render() {
return undefined;
}
}
await render(<UndefinedComponent />, 1);
},
'UndefinedComponent(...): Nothing was returned from render. ' +
'This usually means a return statement is missing. Or, to ' +
'render nothing, return null.',
);
await render(<UndefinedComponent />);
});
});
describe('components that throw errors', function() {
itThrowsWhenRendering(
'a function returning an object',
async render => {

View File

@ -45,10 +45,23 @@ describe('ReactEmptyComponent', () => {
};
});
it('should not produce child DOM nodes for null and false', () => {
describe.each([null, undefined])('when %s', nullORUndefined => {
it('should not throw when rendering', () => {
class Component extends React.Component {
render() {
return nullORUndefined;
}
}
expect(function() {
ReactTestUtils.renderIntoDocument(<Component />);
}).not.toThrowError();
});
it('should not produce child DOM nodes for nullish and false', () => {
class Component1 extends React.Component {
render() {
return null;
return nullORUndefined;
}
}
@ -67,25 +80,18 @@ describe('ReactEmptyComponent', () => {
expect(container2.children.length).toBe(0);
});
it('should still throw when rendering to undefined', () => {
class Component extends React.Component {
render() {}
}
expect(function() {
ReactTestUtils.renderIntoDocument(<Component />);
}).toThrowError(
'Component(...): Nothing was returned from render. This usually means a return statement is missing. ' +
'Or, to render nothing, return null.',
);
});
it('should be able to switch between rendering null and a normal tag', () => {
it('should be able to switch between rendering nullish and a normal tag', () => {
const instance1 = (
<TogglingComponent firstComponent={null} secondComponent={'div'} />
<TogglingComponent
firstComponent={nullORUndefined}
secondComponent={'div'}
/>
);
const instance2 = (
<TogglingComponent firstComponent={'div'} secondComponent={null} />
<TogglingComponent
firstComponent={'div'}
secondComponent={nullORUndefined}
/>
);
ReactTestUtils.renderIntoDocument(instance1);
@ -106,7 +112,10 @@ describe('ReactEmptyComponent', () => {
it('should be able to switch in a list of children', () => {
const instance1 = (
<TogglingComponent firstComponent={null} secondComponent={'div'} />
<TogglingComponent
firstComponent={nullORUndefined}
secondComponent={'div'}
/>
);
ReactTestUtils.renderIntoDocument(
@ -137,10 +146,16 @@ describe('ReactEmptyComponent', () => {
it('should distinguish between a script placeholder and an actual script tag', () => {
const instance1 = (
<TogglingComponent firstComponent={null} secondComponent={'script'} />
<TogglingComponent
firstComponent={nullORUndefined}
secondComponent={'script'}
/>
);
const instance2 = (
<TogglingComponent firstComponent={'script'} secondComponent={null} />
<TogglingComponent
firstComponent={'script'}
secondComponent={nullORUndefined}
/>
);
expect(function() {
@ -165,11 +180,11 @@ describe('ReactEmptyComponent', () => {
it(
'should have findDOMNode return null when multiple layers of composite ' +
'components render to the same null placeholder',
'components render to the same nullish placeholder',
() => {
class GrandChild extends React.Component {
render() {
return null;
return nullORUndefined;
}
}
@ -232,7 +247,7 @@ describe('ReactEmptyComponent', () => {
class Wrapper extends React.Component {
render() {
return this.props.showInner ? <Inner /> : null;
return this.props.showInner ? <Inner /> : nullORUndefined;
}
}
@ -254,9 +269,9 @@ describe('ReactEmptyComponent', () => {
expect(assertions).toBe(3);
});
it('can render null at the top level', () => {
it('can render nullish at the top level', () => {
const div = document.createElement('div');
ReactDOM.render(null, div);
ReactDOM.render(nullORUndefined, div);
expect(div.innerHTML).toBe('');
});
@ -270,7 +285,7 @@ describe('ReactEmptyComponent', () => {
render() {
if (!this.props.visible) {
return null;
return nullORUndefined;
}
return <div>hello world</div>;
@ -301,7 +316,7 @@ describe('ReactEmptyComponent', () => {
it('preserves the dom node during updates', () => {
class Empty extends React.Component {
render() {
return null;
return nullORUndefined;
}
}
@ -317,23 +332,26 @@ describe('ReactEmptyComponent', () => {
expect(noscript2).toBe(null);
});
it('should warn about React.forwardRef that returns undefined', () => {
const Empty = () => {};
it('should not warn about React.forwardRef that returns nullish', () => {
const Empty = () => {
return nullORUndefined;
};
const EmptyForwardRef = React.forwardRef(Empty);
expect(() => {
ReactTestUtils.renderIntoDocument(<EmptyForwardRef />);
}).toThrowError(
'ForwardRef(Empty)(...): Nothing was returned from render.',
);
}).not.toThrowError();
});
it('should warn about React.memo that returns undefined', () => {
const Empty = () => {};
it('should not warn about React.memo that returns nullish', () => {
const Empty = () => {
return nullORUndefined;
};
const EmptyMemo = React.memo(Empty);
expect(() => {
ReactTestUtils.renderIntoDocument(<EmptyMemo />);
}).toThrowError('Empty(...): Nothing was returned from render.');
}).not.toThrowError();
});
});
});

View File

@ -136,7 +136,7 @@ describe('ReactFunctionComponent', () => {
);
});
it('should throw when stateless component returns undefined', () => {
it('should not throw when stateless component returns undefined', () => {
function NotAComponent() {}
expect(function() {
ReactTestUtils.renderIntoDocument(
@ -144,10 +144,7 @@ describe('ReactFunctionComponent', () => {
<NotAComponent />
</div>,
);
}).toThrowError(
'NotAComponent(...): Nothing was returned from render. ' +
'This usually means a return statement is missing. Or, to render nothing, return null.',
);
}).not.toThrowError();
});
it('should throw on string refs in pure functions', () => {

View File

@ -30,42 +30,18 @@ describe('ReactMockedComponent', () => {
MockedComponent.prototype.render = jest.fn();
});
it('should allow a mocked component to be rendered in dev', () => {
it('should allow a mocked component to be rendered', () => {
const container = document.createElement('container');
if (__DEV__) {
ReactDOM.render(<MockedComponent />, container);
} else {
expect(() => ReactDOM.render(<MockedComponent />, container)).toThrow(
'Nothing was returned from render.',
);
}
});
it('should allow a mocked component to be updated in dev', () => {
const container = document.createElement('container');
if (__DEV__) {
ReactDOM.render(<MockedComponent />, container);
} else {
expect(() => ReactDOM.render(<MockedComponent />, container)).toThrow(
'Nothing was returned from render.',
);
}
if (__DEV__) {
ReactDOM.render(<MockedComponent />, container);
} else {
expect(() => ReactDOM.render(<MockedComponent />, container)).toThrow(
'Nothing was returned from render.',
);
}
});
it('should allow a mocked component to be rendered in dev (SSR)', () => {
if (__DEV__) {
ReactDOMServer.renderToString(<MockedComponent />);
} else {
expect(() => ReactDOMServer.renderToString(<MockedComponent />)).toThrow(
'Nothing was returned from render.',
);
}
});
});

View File

@ -242,7 +242,7 @@ module.exports = function(initModules) {
await asyncReactDOMRender(element, cleanContainer, true);
// This gives us the expected text content.
const cleanTextContent =
cleanContainer.lastChild && cleanContainer.lastChild.textContent;
(cleanContainer.lastChild && cleanContainer.lastChild.textContent) || '';
// The only guarantee is that text content has been patched up if needed.
expect(hydratedTextContent).toBe(cleanTextContent);

View File

@ -391,18 +391,6 @@ function createOpenTagMarkup(
return ret;
}
function validateRenderResult(child, type) {
if (child === undefined) {
invariant(
false,
'%s(...): Nothing was returned from render. This usually means a ' +
'return statement is missing. Or, to render nothing, ' +
'return null.',
getComponentNameFromType(type) || 'Component',
);
}
}
function resolve(
child: mixed,
context: Object,
@ -631,7 +619,6 @@ function resolve(
inst.render == null
) {
child = inst;
validateRenderResult(child, Component);
return;
}
}
@ -720,15 +707,6 @@ function resolve(
}
child = inst.render();
if (__DEV__) {
if (child === undefined && inst.render._isMockFunction) {
// This is probably bad practice. Consider warning here and
// deprecating this convenience.
child = null;
}
}
validateRenderResult(child, Component);
let childContext;
if (disableLegacyContext) {
if (__DEV__) {

View File

@ -21,15 +21,7 @@ import {
REACT_PORTAL_TYPE,
REACT_LAZY_TYPE,
} from 'shared/ReactSymbols';
import {
FunctionComponent,
ClassComponent,
HostText,
HostPortal,
ForwardRef,
Fragment,
SimpleMemoComponent,
} from './ReactWorkTags';
import {ClassComponent, HostText, HostPortal, Fragment} from './ReactWorkTags';
import invariant from 'shared/invariant';
import isArray from 'shared/isArray';
import {
@ -1302,36 +1294,6 @@ function ChildReconciler(shouldTrackSideEffects) {
warnOnFunctionType(returnFiber);
}
}
if (typeof newChild === 'undefined' && !isUnkeyedTopLevelFragment) {
// If the new child is undefined, and the return fiber is a composite
// component, throw an error. If Fiber return types are disabled,
// we already threw above.
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 FunctionComponent:
case ForwardRef:
case SimpleMemoComponent: {
invariant(
false,
'%s(...): Nothing was returned from render. This usually means a ' +
'return statement is missing. Or, to render nothing, ' +
'return null.',
getComponentNameFromFiber(returnFiber) || 'Component',
);
}
}
}
// Remaining cases are all treated as empty.
return deleteRemainingChildren(returnFiber, currentFirstChild);

View File

@ -21,15 +21,7 @@ import {
REACT_PORTAL_TYPE,
REACT_LAZY_TYPE,
} from 'shared/ReactSymbols';
import {
FunctionComponent,
ClassComponent,
HostText,
HostPortal,
ForwardRef,
Fragment,
SimpleMemoComponent,
} from './ReactWorkTags';
import {ClassComponent, HostText, HostPortal, Fragment} from './ReactWorkTags';
import invariant from 'shared/invariant';
import isArray from 'shared/isArray';
import {
@ -1302,36 +1294,6 @@ function ChildReconciler(shouldTrackSideEffects) {
warnOnFunctionType(returnFiber);
}
}
if (typeof newChild === 'undefined' && !isUnkeyedTopLevelFragment) {
// If the new child is undefined, and the return fiber is a composite
// component, throw an error. If Fiber return types are disabled,
// we already threw above.
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 FunctionComponent:
case ForwardRef:
case SimpleMemoComponent: {
invariant(
false,
'%s(...): Nothing was returned from render. This usually means a ' +
'return statement is missing. Or, to render nothing, ' +
'return null.',
getComponentNameFromFiber(returnFiber) || 'Component',
);
}
}
}
// Remaining cases are all treated as empty.
return deleteRemainingChildren(returnFiber, currentFirstChild);

View File

@ -554,16 +554,6 @@ function shouldConstruct(Component) {
return Component.prototype && Component.prototype.isReactComponent;
}
function invalidRenderResult(type: any): void {
invariant(
false,
'%s(...): Nothing was returned from render. This usually means a ' +
'return statement is missing. Or, to render nothing, ' +
'return null.',
getComponentNameFromType(type) || 'Component',
);
}
function renderWithHooks<Props, SecondArg>(
request: Request,
task: Task,
@ -574,11 +564,7 @@ function renderWithHooks<Props, SecondArg>(
const componentIdentity = {};
prepareToUseHooks(componentIdentity);
const result = Component(props, secondArg);
const children = finishHooks(Component, props, result, secondArg);
if (children === undefined) {
invalidRenderResult(Component);
}
return children;
return finishHooks(Component, props, result, secondArg);
}
function finishClassComponent(
@ -589,13 +575,6 @@ function finishClassComponent(
props: any,
): ReactNodeList {
const nextChildren = instance.render();
if (nextChildren === undefined) {
if (__DEV__ && instance.render._isMockFunction) {
// We allow auto-mocks to proceed as if they're returning null.
} else {
invalidRenderResult(Component);
}
}
if (__DEV__) {
if (instance.props !== props) {

View File

@ -46,18 +46,12 @@ describe('ReactError', () => {
);
});
// @gate build === "production"
it('should serialize arguments', () => {
function Oops() {
return;
}
Oops.displayName = '#wtf';
const container = document.createElement('div');
expect(() => ReactDOM.render(<Oops />, container)).toThrowError(
'Minified React error #152; visit ' +
'https://reactjs.org/docs/error-decoder.html?invariant=152&args[]=%23wtf' +
' for the full message or use the non-minified dev environment' +
' for full errors and additional helpful warnings.',
);
expect(() => ReactDOM.render(<Oops />, container)).not.toThrowError();
});
});