Skip to content

Commit

Permalink
Include the function name for context on invalid function child (#28362)
Browse files Browse the repository at this point in the history 
Also warn for symbols.

It's weird because for objects we throw a hard error but functions we do
a dev only check. Mainly because we have an object branch anyway.

In the object branch we have some built-ins that have bad errors like
forwardRef and memo but since they're going to become functions later, I
didn't bother updating those. Once they're functions those names will be
part of this.
  • Loading branch information
@sebmarkbage
sebmarkbage committed Feb 17, 2024
1 parent fef30c2 commit c1fd2a9
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 28 deletions.
15 changes: 10 additions & 5 deletions packages/react-dom/src/__tests__/ReactComponent-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -635,8 +635,9 @@ describe('ReactComponent', () => {
});
}).toErrorDev(
'Warning: Functions are not valid as a React child. This may happen if ' +
'you return a Component instead of <Component /> from render. ' +
'you return Foo instead of <Foo /> from render. ' +
'Or maybe you meant to call this function rather than return it.\n' +
' <Foo>{Foo}</Foo>\n' +
' in Foo (at **)',
);
});
Expand All @@ -656,8 +657,9 @@ describe('ReactComponent', () => {
});
}).toErrorDev(
'Warning: Functions are not valid as a React child. This may happen if ' +
'you return a Component instead of <Component /> from render. ' +
'you return Foo instead of <Foo /> from render. ' +
'Or maybe you meant to call this function rather than return it.\n' +
' <Foo>{Foo}</Foo>\n' +
' in Foo (at **)',
);
});
Expand All @@ -678,8 +680,9 @@ describe('ReactComponent', () => {
});
}).toErrorDev(
'Warning: Functions are not valid as a React child. This may happen if ' +
'you return a Component instead of <Component /> from render. ' +
'you return Foo instead of <Foo /> from render. ' +
'Or maybe you meant to call this function rather than return it.\n' +
' <span>{Foo}</span>\n' +
' in span (at **)\n' +
' in div (at **)\n' +
' in Foo (at **)',
Expand Down Expand Up @@ -730,13 +733,15 @@ describe('ReactComponent', () => {
});
}).toErrorDev([
'Warning: Functions are not valid as a React child. This may happen if ' +
'you return a Component instead of <Component /> from render. ' +
'you return Foo instead of <Foo /> from render. ' +
'Or maybe you meant to call this function rather than return it.\n' +
' <div>{Foo}</div>\n' +
' in div (at **)\n' +
' in Foo (at **)',
'Warning: Functions are not valid as a React child. This may happen if ' +
'you return a Component instead of <Component /> from render. ' +
'you return Foo instead of <Foo /> from render. ' +
'Or maybe you meant to call this function rather than return it.\n' +
' <span>{Foo}</span>\n' +
' in span (at **)\n' +
' in div (at **)\n' +
' in Foo (at **)',
Expand Down
19 changes: 17 additions & 2 deletions packages/react-dom/src/__tests__/ReactDOMRoot-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -487,8 +487,23 @@ describe('ReactDOMRoot', () => {
});
}).toErrorDev(
'Functions are not valid as a React child. ' +
'This may happen if you return a Component instead of <Component /> from render. ' +
'Or maybe you meant to call this function rather than return it.',
'This may happen if you return Component instead of <Component /> from render. ' +
'Or maybe you meant to call this function rather than return it.\n' +
' root.render(Component)',
{withoutStack: true},
);
});

it('warns when given a symbol', () => {
const root = ReactDOMClient.createRoot(document.createElement('div'));

expect(() => {
ReactDOM.flushSync(() => {
root.render(Symbol('foo'));
});
}).toErrorDev(
'Symbols are not valid as a React child.\n' +
' root.render(Symbol(foo))',
{withoutStack: true},
);
});
Expand Down
5 changes: 3 additions & 2 deletions packages/react-dom/src/__tests__/ReactLegacyMount-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,9 @@ describe('ReactMount', () => {

expect(() => ReactTestUtils.renderIntoDocument(Component)).toErrorDev(
'Functions are not valid as a React child. ' +
'This may happen if you return a Component instead of <Component /> from render. ' +
'Or maybe you meant to call this function rather than return it.',
'This may happen if you return Component instead of <Component /> from render. ' +
'Or maybe you meant to call this function rather than return it.\n' +
' root.render(Component)',
{withoutStack: true},
);
});
Expand Down
96 changes: 82 additions & 14 deletions packages/react-reconciler/src/ReactChildFiber.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,13 @@ import {
REACT_LAZY_TYPE,
REACT_CONTEXT_TYPE,
} from 'shared/ReactSymbols';
import {ClassComponent, HostText, HostPortal, Fragment} from './ReactWorkTags';
import {
ClassComponent,
HostRoot,
HostText,
HostPortal,
Fragment,
} from './ReactWorkTags';
import isArray from 'shared/isArray';
import {checkPropStringCoercion} from 'shared/CheckStringCoercion';

Expand Down Expand Up @@ -79,6 +85,7 @@ let didWarnAboutGenerators;
let didWarnAboutStringRefs;
let ownerHasKeyUseWarning;
let ownerHasFunctionTypeWarning;
let ownerHasSymbolTypeWarning;
let warnForMissingKey = (child: mixed, returnFiber: Fiber) => {};

if (__DEV__) {
Expand All @@ -93,6 +100,7 @@ if (__DEV__) {
*/
ownerHasKeyUseWarning = ({}: {[string]: boolean});
ownerHasFunctionTypeWarning = ({}: {[string]: boolean});
ownerHasSymbolTypeWarning = ({}: {[string]: boolean});

warnForMissingKey = (child: mixed, returnFiber: Fiber) => {
if (child === null || typeof child !== 'object') {
Expand Down Expand Up @@ -267,20 +275,68 @@ function throwOnInvalidObjectType(returnFiber: Fiber, newChild: Object) {
);
}

function warnOnFunctionType(returnFiber: Fiber) {
function warnOnFunctionType(returnFiber: Fiber, invalidChild: Function) {
if (__DEV__) {
const componentName = getComponentNameFromFiber(returnFiber) || 'Component';
const parentName = getComponentNameFromFiber(returnFiber) || 'Component';

if (ownerHasFunctionTypeWarning[componentName]) {
if (ownerHasFunctionTypeWarning[parentName]) {
return;
}
ownerHasFunctionTypeWarning[componentName] = true;
ownerHasFunctionTypeWarning[parentName] = true;

const name = invalidChild.displayName || invalidChild.name || 'Component';

if (returnFiber.tag === HostRoot) {
console.error(
'Functions are not valid as a React child. This may happen if ' +
'you return %s instead of <%s /> from render. ' +
'Or maybe you meant to call this function rather than return it.\n' +
' root.render(%s)',
name,
name,
name,
);
} else {
console.error(
'Functions are not valid as a React child. This may happen if ' +
'you return %s instead of <%s /> from render. ' +
'Or maybe you meant to call this function rather than return it.\n' +
' <%s>{%s}</%s>',
name,
name,
parentName,
name,
parentName,
);
}
}
}

console.error(
'Functions are not valid as a React child. This may happen if ' +
'you return a Component instead of <Component /> from render. ' +
'Or maybe you meant to call this function rather than return it.',
);
function warnOnSymbolType(returnFiber: Fiber, invalidChild: symbol) {
if (__DEV__) {
const parentName = getComponentNameFromFiber(returnFiber) || 'Component';

if (ownerHasSymbolTypeWarning[parentName]) {
return;
}
ownerHasSymbolTypeWarning[parentName] = true;

// eslint-disable-next-line react-internal/safe-string-coercion
const name = String(invalidChild);

if (returnFiber.tag === HostRoot) {
console.error(
'Symbols are not valid as a React child.\n' + ' root.render(%s)',
name,
);
} else {
console.error(
'Symbols are not valid as a React child.\n' + ' <%s>%s</%s>',
parentName,
name,
parentName,
);
}
}
}

Expand Down Expand Up @@ -656,7 +712,10 @@ function createChildReconciler(

if (__DEV__) {
if (typeof newChild === 'function') {
warnOnFunctionType(returnFiber);
warnOnFunctionType(returnFiber, newChild);
}
if (typeof newChild === 'symbol') {
warnOnSymbolType(returnFiber, newChild);
}
}

Expand Down Expand Up @@ -778,7 +837,10 @@ function createChildReconciler(

if (__DEV__) {
if (typeof newChild === 'function') {
warnOnFunctionType(returnFiber);
warnOnFunctionType(returnFiber, newChild);
}
if (typeof newChild === 'symbol') {
warnOnSymbolType(returnFiber, newChild);
}
}

Expand Down Expand Up @@ -894,7 +956,10 @@ function createChildReconciler(

if (__DEV__) {
if (typeof newChild === 'function') {
warnOnFunctionType(returnFiber);
warnOnFunctionType(returnFiber, newChild);
}
if (typeof newChild === 'symbol') {
warnOnSymbolType(returnFiber, newChild);
}
}

Expand Down Expand Up @@ -1621,7 +1686,10 @@ function createChildReconciler(

if (__DEV__) {
if (typeof newChild === 'function') {
warnOnFunctionType(returnFiber);
warnOnFunctionType(returnFiber, newChild);
}
if (typeof newChild === 'symbol') {
warnOnSymbolType(returnFiber, newChild);
}
}

Expand Down
30 changes: 25 additions & 5 deletions packages/react-server/src/ReactFizzServer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2137,6 +2137,27 @@ function validateIterable(iterable, iteratorFn: Function): void {
}
}

function warnOnFunctionType(invalidChild: Function) {
if (__DEV__) {
const name = invalidChild.displayName || invalidChild.name || 'Component';
console.error(
'Functions are not valid as a React child. This may happen if ' +
'you return %s instead of <%s /> from render. ' +
'Or maybe you meant to call this function rather than return it.',
name,
name,
);
}
}

function warnOnSymbolType(invalidChild: symbol) {
if (__DEV__) {
// eslint-disable-next-line react-internal/safe-string-coercion
const name = String(invalidChild);
console.error('Symbols are not valid as a React child.\n' + ' %s', name);
}
}

// This function by it self renders a node and consumes the task by mutating it
// to update the current execution state.
function renderNodeDestructive(
Expand Down Expand Up @@ -2329,11 +2350,10 @@ function renderNodeDestructive(

if (__DEV__) {
if (typeof node === 'function') {
console.error(
'Functions are not valid as a React child. This may happen if ' +
'you return a Component instead of <Component /> from render. ' +
'Or maybe you meant to call this function rather than return it.',
);
warnOnFunctionType(node);
}
if (typeof node === 'symbol') {
warnOnSymbolType(node);
}
}
}
Expand Down

0 comments on commit c1fd2a9

Please sign in to comment.