Skip to content

Commit

Permalink
Remove method name prefix from warnings and errors
Browse files Browse the repository at this point in the history
This pattern is a petpeeve of mine. I don't consider this best practice
and so most don't have these prefixes. Very inconsistent.

At best this is useless and noisey that you have to parse because the
information is also in the stack trace.

At worse these are misleading because they'rehighlighting something
internal (like validateDOMNesting) which even suggests an internal bug.
Even the ones public to React aren't necessarily what you called because
you might be calling a wrapper around it.

That would be properly reflected in a stack trace - which can also properly
ignore list so that the first stack you see is your callsite,

Which might be like render() in react-testing-library rather than createRoot()
for example.
  • Loading branch information
sebmarkbage committed Feb 23, 2024
1 parent 66c8346 commit 97b4970
Show file tree
Hide file tree
Showing 39 changed files with 188 additions and 215 deletions.
12 changes: 4 additions & 8 deletions packages/react-dom-bindings/src/client/validateDOMNesting.js
Original file line number Diff line number Diff line change
Expand Up @@ -477,15 +477,14 @@ function validateDOMNesting(
'the browser.';
}
console.error(
'validateDOMNesting(...): %s cannot appear as a child of <%s>.%s',
'%s cannot appear as a child of <%s>.%s',
tagDisplayName,
ancestorTag,
info,
);
} else {
console.error(
'validateDOMNesting(...): %s cannot appear as a descendant of ' +
'<%s>.',
'%s cannot appear as a descendant of ' + '<%s>.',
tagDisplayName,
ancestorTag,
);
Expand All @@ -507,13 +506,10 @@ function validateTextNesting(childText: string, parentTag: string): void {
didWarn[warnKey] = true;

if (/\S/.test(childText)) {
console.error(
'validateDOMNesting(...): Text nodes cannot appear as a child of <%s>.',
parentTag,
);
console.error('Text nodes cannot appear as a child of <%s>.', parentTag);
} else {
console.error(
'validateDOMNesting(...): Whitespace text nodes cannot appear as a child of <%s>. ' +
'Whitespace text nodes cannot appear as a child of <%s>. ' +
"Make sure you don't have any extra whitespace between tags on " +
'each line of your source code.',
parentTag,
Expand Down
26 changes: 13 additions & 13 deletions packages/react-dom/src/__tests__/ReactCompositeComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -879,7 +879,7 @@ describe('ReactCompositeComponent', () => {
root.render(<Foo idx="qwe" />);
});
}).toErrorDev(
'Foo(...): When calling super() in `Foo`, make sure to pass ' +
'When calling super() in `Foo`, make sure to pass ' +
"up the same props that your component's constructor was passed.",
);
});
Expand Down Expand Up @@ -1233,14 +1233,14 @@ describe('ReactCompositeComponent', () => {
}).toErrorDev([
// Expect two errors because invokeGuardedCallback will dispatch an error event,
// Causing the warning to be logged again.
'Warning: RenderTextInvalidConstructor(...): No `render` method found on the returned component instance: ' +
'Warning: No `render` method found on the RenderTextInvalidConstructor instance: ' +
'did you accidentally return an object from the constructor?',
'Warning: RenderTextInvalidConstructor(...): No `render` method found on the returned component instance: ' +
'Warning: No `render` method found on the RenderTextInvalidConstructor instance: ' +
'did you accidentally return an object from the constructor?',
// And then two more because we retry errors.
'Warning: RenderTextInvalidConstructor(...): No `render` method found on the returned component instance: ' +
'Warning: No `render` method found on the RenderTextInvalidConstructor instance: ' +
'did you accidentally return an object from the constructor?',
'Warning: RenderTextInvalidConstructor(...): No `render` method found on the returned component instance: ' +
'Warning: No `render` method found on the RenderTextInvalidConstructor instance: ' +
'did you accidentally return an object from the constructor?',
]);
});
Expand Down Expand Up @@ -1280,16 +1280,16 @@ describe('ReactCompositeComponent', () => {
}).toErrorDev([
// Expect two errors because invokeGuardedCallback will dispatch an error event,
// Causing the warning to be logged again.
'Warning: RenderTestUndefinedRender(...): No `render` method found on the returned ' +
'component instance: you may have forgotten to define `render`.',
'Warning: RenderTestUndefinedRender(...): No `render` method found on the returned ' +
'component instance: you may have forgotten to define `render`.',
'Warning: No `render` method found on the RenderTestUndefinedRender instance: ' +
'you may have forgotten to define `render`.',
'Warning: No `render` method found on the RenderTestUndefinedRender instance: ' +
'you may have forgotten to define `render`.',

// And then two more because we retry errors.
'Warning: RenderTestUndefinedRender(...): No `render` method found on the returned ' +
'component instance: you may have forgotten to define `render`.',
'Warning: RenderTestUndefinedRender(...): No `render` method found on the returned ' +
'component instance: you may have forgotten to define `render`.',
'Warning: No `render` method found on the RenderTestUndefinedRender instance: ' +
'you may have forgotten to define `render`.',
'Warning: No `render` method found on the RenderTestUndefinedRender instance: ' +
'you may have forgotten to define `render`.',
]);
});

Expand Down
12 changes: 6 additions & 6 deletions packages/react-dom/src/__tests__/ReactDOM-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ describe('ReactDOM', () => {
expect(() => {
ReactDOM.render(<A />, myDiv, 'no');
}).toErrorDev(
'render(...): Expected the last optional `callback` argument to be ' +
'Expected the last optional `callback` argument to be ' +
'a function. Instead received: no.',
);
}).toThrowError(
Expand All @@ -195,7 +195,7 @@ describe('ReactDOM', () => {
expect(() => {
ReactDOM.render(<A />, myDiv, {foo: 'bar'});
}).toErrorDev(
'render(...): Expected the last optional `callback` argument to be ' +
'Expected the last optional `callback` argument to be ' +
'a function. Instead received: [object Object].',
);
}).toThrowError(
Expand All @@ -207,7 +207,7 @@ describe('ReactDOM', () => {
expect(() => {
ReactDOM.render(<A />, myDiv, new Foo());
}).toErrorDev(
'render(...): Expected the last optional `callback` argument to be ' +
'Expected the last optional `callback` argument to be ' +
'a function. Instead received: [object Object].',
);
}).toThrowError(
Expand Down Expand Up @@ -236,7 +236,7 @@ describe('ReactDOM', () => {
expect(() => {
ReactDOM.render(<A />, myDiv, 'no');
}).toErrorDev(
'render(...): Expected the last optional `callback` argument to be ' +
'Expected the last optional `callback` argument to be ' +
'a function. Instead received: no.',
);
}).toThrowError(
Expand All @@ -249,7 +249,7 @@ describe('ReactDOM', () => {
expect(() => {
ReactDOM.render(<A />, myDiv, {foo: 'bar'});
}).toErrorDev(
'render(...): Expected the last optional `callback` argument to be ' +
'Expected the last optional `callback` argument to be ' +
'a function. Instead received: [object Object].',
);
}).toThrowError(
Expand All @@ -262,7 +262,7 @@ describe('ReactDOM', () => {
expect(() => {
ReactDOM.render(<A />, myDiv, new Foo());
}).toErrorDev(
'render(...): Expected the last optional `callback` argument to be ' +
'Expected the last optional `callback` argument to be ' +
'a function. Instead received: [object Object].',
);
}).toThrowError(
Expand Down
14 changes: 7 additions & 7 deletions packages/react-dom/src/__tests__/ReactDOMComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2188,7 +2188,7 @@ describe('ReactDOMComponent', () => {
);
});
}).toErrorDev([
'Warning: validateDOMNesting(...): <tr> cannot appear as a child of ' +
'Warning: <tr> cannot appear as a child of ' +
'<div>.' +
'\n in tr (at **)' +
'\n in div (at **)',
Expand All @@ -2208,7 +2208,7 @@ describe('ReactDOMComponent', () => {
);
});
}).toErrorDev(
'Warning: validateDOMNesting(...): <p> cannot appear as a descendant ' +
'Warning: <p> cannot appear as a descendant ' +
'of <p>.' +
// There is no outer `p` here because root container is not part of the stack.
'\n in p (at **)' +
Expand Down Expand Up @@ -2241,20 +2241,20 @@ describe('ReactDOMComponent', () => {
root.render(<Foo />);
});
}).toErrorDev([
'Warning: validateDOMNesting(...): <tr> cannot appear as a child of ' +
'Warning: <tr> cannot appear as a child of ' +
'<table>. Add a <tbody>, <thead> or <tfoot> to your code to match the DOM tree generated ' +
'by the browser.' +
'\n in tr (at **)' +
'\n in Row (at **)' +
'\n in table (at **)' +
'\n in Foo (at **)',
'Warning: validateDOMNesting(...): Text nodes cannot appear as a ' +
'Warning: Text nodes cannot appear as a ' +
'child of <tr>.' +
'\n in tr (at **)' +
'\n in Row (at **)' +
'\n in table (at **)' +
'\n in Foo (at **)',
'Warning: validateDOMNesting(...): Whitespace text nodes cannot ' +
'Warning: Whitespace text nodes cannot ' +
"appear as a child of <table>. Make sure you don't have any extra " +
'whitespace between tags on each line of your source code.' +
'\n in table (at **)' +
Expand Down Expand Up @@ -2283,7 +2283,7 @@ describe('ReactDOMComponent', () => {
root.render(<Foo> </Foo>);
});
}).toErrorDev([
'Warning: validateDOMNesting(...): Whitespace text nodes cannot ' +
'Warning: Whitespace text nodes cannot ' +
"appear as a child of <table>. Make sure you don't have any extra " +
'whitespace between tags on each line of your source code.' +
'\n in table (at **)' +
Expand Down Expand Up @@ -2311,7 +2311,7 @@ describe('ReactDOMComponent', () => {
);
});
}).toErrorDev([
'Warning: validateDOMNesting(...): Text nodes cannot appear as a ' +
'Warning: Text nodes cannot appear as a ' +
'child of <tr>.' +
'\n in tr (at **)' +
'\n in Row (at **)' +
Expand Down
20 changes: 10 additions & 10 deletions packages/react-dom/src/__tests__/ReactDOMFloat-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ describe('ReactDOMFloat', () => {
}).toErrorDev(
[
'Cannot render <noscript> outside the main document. Try moving it into the root <head> tag.',
'Warning: validateDOMNesting(...): <noscript> cannot appear as a child of <#document>.',
'Warning: <noscript> cannot appear as a child of <#document>.',
],
{withoutStack: 1},
);
Expand All @@ -538,7 +538,7 @@ describe('ReactDOMFloat', () => {
await waitForAll([]);
}).toErrorDev([
'Cannot render <template> outside the main document. Try moving it into the root <head> tag.',
'Warning: validateDOMNesting(...): <template> cannot appear as a child of <html>.',
'Warning: <template> cannot appear as a child of <html>.',
]);

await expect(async () => {
Expand All @@ -551,7 +551,7 @@ describe('ReactDOMFloat', () => {
await waitForAll([]);
}).toErrorDev([
'Cannot render a <style> outside the main document without knowing its precedence and a unique href key. React can hoist and deduplicate <style> tags if you provide a `precedence` prop along with an `href` prop that does not conflic with the `href` values used in any other hoisted <style> or <link rel="stylesheet" ...> tags. Note that hoisting <style> tags is considered an advanced feature that most will not use directly. Consider moving the <style> tag to the <head> or consider adding a `precedence="default"` and `href="some unique resource identifier"`, or move the <style> to the <style> tag.',
'Warning: validateDOMNesting(...): <style> cannot appear as a child of <html>.',
'Warning: <style> cannot appear as a child of <html>.',
]);

await expect(async () => {
Expand All @@ -574,7 +574,7 @@ describe('ReactDOMFloat', () => {
}).toErrorDev(
[
'Cannot render a <link rel="stylesheet" /> outside the main document without knowing its precedence. Consider adding precedence="default" or moving it into the root <head> tag.',
'Warning: validateDOMNesting(...): <link> cannot appear as a child of <#document>.',
'Warning: <link> cannot appear as a child of <#document>.',
],
{withoutStack: 1},
);
Expand All @@ -591,7 +591,7 @@ describe('ReactDOMFloat', () => {
await waitForAll([]);
}).toErrorDev([
'Cannot render a sync or defer <script> outside the main document without knowing its order. Try adding async="" or moving it into the root <head> tag.',
'Warning: validateDOMNesting(...): <script> cannot appear as a child of <html>.',
'Warning: <script> cannot appear as a child of <html>.',
]);

await expect(async () => {
Expand Down Expand Up @@ -2552,11 +2552,11 @@ body {
'Cannot render a <style> outside the main document if it has an `itemProp` prop. `itemProp` suggests the tag belongs to an `itemScope` which can appear anywhere in the DOM. If you were intending for React to hoist this <style> remove the `itemProp` prop. Otherwise, try moving this tag into the <head> or <body> of the Document.',
'Cannot render a <link> outside the main document if it has an `itemProp` prop. `itemProp` suggests the tag belongs to an `itemScope` which can appear anywhere in the DOM. If you were intending for React to hoist this <link> remove the `itemProp` prop. Otherwise, try moving this tag into the <head> or <body> of the Document.',
'Cannot render a <script> outside the main document if it has an `itemProp` prop. `itemProp` suggests the tag belongs to an `itemScope` which can appear anywhere in the DOM. If you were intending for React to hoist this <script> remove the `itemProp` prop. Otherwise, try moving this tag into the <head> or <body> of the Document.',
'validateDOMNesting(...): <meta> cannot appear as a child of <html>',
'validateDOMNesting(...): <title> cannot appear as a child of <html>',
'validateDOMNesting(...): <style> cannot appear as a child of <html>',
'validateDOMNesting(...): <link> cannot appear as a child of <html>',
'validateDOMNesting(...): <script> cannot appear as a child of <html>',
'<meta> cannot appear as a child of <html>',
'<title> cannot appear as a child of <html>',
'<style> cannot appear as a child of <html>',
'<link> cannot appear as a child of <html>',
'<script> cannot appear as a child of <html>',
]);
});

Expand Down
2 changes: 1 addition & 1 deletion packages/react-dom/src/__tests__/ReactDOMForm-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ describe('ReactDOMForm', () => {
);
});
}).toErrorDev([
'Warning: validateDOMNesting(...): <form> cannot appear as a descendant of <form>.' +
'Warning: <form> cannot appear as a descendant of <form>.' +
'\n in form (at **)' +
'\n in form (at **)',
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ describe('ReactDOMComponentTree', () => {
const anotherComponent = <div />;
const instance = ReactDOM.render(component, container);
expect(() => ReactDOM.render(anotherComponent, instance)).toErrorDev(
'render(...): Replacing React-rendered children with a new root ' +
'Replacing React-rendered children with a new root ' +
'component. If you intended to update the children of this node, ' +
'you should instead have the existing children update their state ' +
'and render the new components instead of calling ReactDOM.render.',
Expand Down
6 changes: 3 additions & 3 deletions packages/react-dom/src/__tests__/ReactDOMLegacyFiber-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1200,7 +1200,7 @@ describe('ReactDOMLegacyFiber', () => {
expect(() =>
ReactDOM.render(<div key="2">baz</div>, container),
).toErrorDev(
'render(...): ' +
'' +
'It looks like the React-rendered content of this container was ' +
'removed without using React. This is not supported and will ' +
'cause errors. Instead, call ReactDOM.unmountComponentAtNode ' +
Expand All @@ -1218,7 +1218,7 @@ describe('ReactDOMLegacyFiber', () => {
// then we mess with the DOM before an update
container.innerHTML = '<div>MEOW.</div>';
expect(() => ReactDOM.render(<div>baz</div>, container)).toErrorDev(
'render(...): ' +
'' +
'It looks like the React-rendered content of this container was ' +
'removed without using React. This is not supported and will ' +
'cause errors. Instead, call ReactDOM.unmountComponentAtNode ' +
Expand All @@ -1235,7 +1235,7 @@ describe('ReactDOMLegacyFiber', () => {
// then we mess with the DOM before an update
container.innerHTML = '';
expect(() => ReactDOM.render(<div>baz</div>, container)).toErrorDev(
'render(...): ' +
'' +
'It looks like the React-rendered content of this container was ' +
'removed without using React. This is not supported and will ' +
'cause errors. Instead, call ReactDOM.unmountComponentAtNode ' +
Expand Down
4 changes: 2 additions & 2 deletions packages/react-dom/src/__tests__/ReactDOMOption-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe('ReactDOMOption', () => {
expect(() => {
node = ReactTestUtils.renderIntoDocument(el);
}).toErrorDev(
'validateDOMNesting(...): <div> cannot appear as a child of <option>.\n' +
'<div> cannot appear as a child of <option>.\n' +
' in div (at **)\n' +
' in option (at **)',
);
Expand Down Expand Up @@ -263,7 +263,7 @@ describe('ReactDOMOption', () => {
[
'Warning: Text content did not match. Server: "FooBaz" Client: "Foo"',
'Warning: An error occurred during hydration. The server HTML was replaced with client content in <div>',
'Warning: validateDOMNesting(...): <div> cannot appear as a child of <option>',
'Warning: <div> cannot appear as a child of <option>',
],
{withoutStack: 1},
);
Expand Down
12 changes: 6 additions & 6 deletions packages/react-dom/src/__tests__/ReactDOMRoot-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ describe('ReactDOMRoot', () => {
const callback = jest.fn();
const root = ReactDOMClient.createRoot(container);
expect(() => root.render(<div>Hi</div>, callback)).toErrorDev(
'render(...): does not support the second callback argument. ' +
'does not support the second callback argument. ' +
'To execute a side effect after rendering, declare it in a component body with useEffect().',
{withoutStack: true},
);
Expand Down Expand Up @@ -115,7 +115,7 @@ describe('ReactDOMRoot', () => {
const root = ReactDOMClient.createRoot(container);
root.render(<div>Hi</div>);
expect(() => root.unmount(callback)).toErrorDev(
'unmount(...): does not support a callback argument. ' +
'does not support a callback argument. ' +
'To execute a side effect after rendering, declare it in a component body with useEffect().',
{withoutStack: true},
);
Expand Down Expand Up @@ -199,7 +199,7 @@ describe('ReactDOMRoot', () => {
it('throws a good message on invalid containers', () => {
expect(() => {
ReactDOMClient.createRoot(<div>Hi</div>);
}).toThrow('createRoot(...): Target container is not a DOM element.');
}).toThrow('Target container is not a DOM element.');
});

it('warns when creating two roots managing the same container', () => {
Expand Down Expand Up @@ -253,7 +253,7 @@ describe('ReactDOMRoot', () => {
expect(() => {
root.render(<div>Hi</div>);
}).toErrorDev(
'render(...): It looks like the React-rendered content of the ' +
'It looks like the React-rendered content of the ' +
'root container was removed without using React. This is not ' +
'supported and will cause errors. Instead, call ' +
"root.unmount() to empty a root's container.",
Expand Down Expand Up @@ -446,10 +446,10 @@ describe('ReactDOMRoot', () => {
const commentNode = div.childNodes[0];

expect(() => ReactDOMClient.createRoot(commentNode)).toThrow(
'createRoot(...): Target container is not a DOM element.',
'Target container is not a DOM element.',
);
expect(() => ReactDOMClient.hydrateRoot(commentNode)).toThrow(
'hydrateRoot(...): Target container is not a DOM element.',
'Target container is not a DOM element.',
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ describe('ReactDOMServerLifecycles', () => {
'<div>1-2</div>',
);
}).toErrorDev(
'Warning: setState(...): Can only update a mounting component. This ' +
'Warning: Can only update a mounting component. This ' +
'usually means you called setState() outside componentWillMount() on ' +
'the server. This is a no-op.\n\n' +
'Please check the code for the Outer component.',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,7 @@ describe('ReactFunctionComponent', () => {
root.render(<FunctionComponentWithChildContext name="A" />);
});
}).toErrorDev(
'FunctionComponentWithChildContext(...): childContextTypes cannot ' +
'be defined on a function component.',
'childContextTypes cannot ' + 'be defined on a function component.',
);
});

Expand Down
Loading

0 comments on commit 97b4970

Please sign in to comment.