Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove Warning: prefix and toString on console Arguments #29839

Merged
merged 4 commits into from
Jun 10, 2024
Merged

Remove Warning: prefix and toString on console Arguments #29839

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d27c5f3
Remove Warning: prefix and toString on console Arguments
sebmarkbage Jun 10, 2024
ce57244
Update test assertions
sebmarkbage Jun 10, 2024
d19c28b
Ignore both versions in DevTools test that might assert on multiple R…
sebmarkbage Jun 10, 2024
1c844ea
Fix ignore that now looks like error logging
sebmarkbage Jun 10, 2024
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
3 changes: 3 additions & 0 deletions packages/internal-test-utils/shouldIgnoreConsoleError.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ module.exports = function shouldIgnoreConsoleError(format, args) {
args[0] != null &&
((typeof args[0] === 'object' &&
typeof args[0].message === 'string' &&
// This specific log has the same signature as error logging.
// The trick is to get rid of this whole file.
!format.includes('Failed to serialize an action') &&
typeof args[0].stack === 'string') ||
(typeof args[0] === 'string' &&
args[0].indexOf('An error occurred in ') === 0))
Expand Down
2 changes: 1 addition & 1 deletion packages/react-cache/src/__tests__/ReactCacheOld-test.internal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ describe('ReactCache', () => {
await waitForAll(['App', 'Loading...']);
}).toErrorDev([
'Invalid key type. Expected a string, number, symbol, or ' +
'boolean, but instead received: Hi,100\n\n' +
"boolean, but instead received: [ 'Hi', 100 ]\n\n" +
'To use non-primitive values as keys, you must pass a hash ' +
'function as the second argument to createResource().',
]);
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/__tests__/inspectedElement-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2556,7 +2556,7 @@ describe('InspectedElement', () => {
};

await withErrorsOrWarningsIgnored(
['Warning: Each child in a list should have a unique "key" prop.'],
['Each child in a list should have a unique "key" prop.'],
async () => {
await utils.actAsync(() =>
render(<Example repeatWarningCount={1} />),
Expand Down
21 changes: 10 additions & 11 deletions packages/react-devtools-shared/src/__tests__/setupTests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,23 +154,22 @@ beforeEach(() => {

const originalConsoleError = console.error;
console.error = (...args) => {
const firstArg = args[0];
if (
firstArg === 'Warning: React instrumentation encountered an error: %s'
) {
let firstArg = args[0];
if (typeof firstArg === 'string' && firstArg.startsWith('Warning: ')) {
// Older React versions might use the Warning: prefix. I'm not sure
// if they use this code path.
firstArg = firstArg.slice(9);
}
if (firstArg === 'React instrumentation encountered an error: %s') {
// Rethrow errors from React.
throw args[1];
} else if (
typeof firstArg === 'string' &&
(firstArg.startsWith(
"Warning: It looks like you're using the wrong act()",
) ||
(firstArg.startsWith("It looks like you're using the wrong act()") ||
firstArg.startsWith(
'Warning: The current testing environment is not configured to support act',
'The current testing environment is not configured to support act',
) ||
firstArg.startsWith(
'Warning: You seem to have overlapping act() calls',
))
firstArg.startsWith('You seem to have overlapping act() calls'))
) {
// DevTools intentionally wraps updates with acts from both DOM and test-renderer,
// since test updates are expected to impact both renderers.
Expand Down
4 changes: 2 additions & 2 deletions packages/react-devtools-shared/src/__tests__/store-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1927,7 +1927,7 @@ describe('Store', () => {
}

withErrorsOrWarningsIgnored(
['Warning: Each child in a list should have a unique "key" prop'],
['Each child in a list should have a unique "key" prop'],
() => {
act(() => render(<Example />));
},
Expand All @@ -1952,7 +1952,7 @@ describe('Store', () => {
}

withErrorsOrWarningsIgnored(
['Warning: Each child in a list should have a unique "key" prop'],
['Each child in a list should have a unique "key" prop'],
() => {
act(() => render(<Example />));
},
Expand Down
12 changes: 11 additions & 1 deletion packages/react-devtools-shell/src/app/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,18 @@ ignoreErrors([
'Warning: %s is deprecated in StrictMode.', // findDOMNode
'Warning: ReactDOM.render was removed in React 19',
'Warning: react-test-renderer is deprecated',
// Ignore prefixed and not prefixed since I don't know which
// React versions are being tested by this code.
'Legacy context API',
'Unsafe lifecycle methods',
'%s is deprecated in StrictMode.', // findDOMNode
'ReactDOM.render was removed in React 19',
'react-test-renderer is deprecated',
]);
ignoreWarnings([
'Warning: componentWillReceiveProps has been renamed',
'componentWillReceiveProps has been renamed',
]);
ignoreWarnings(['Warning: componentWillReceiveProps has been renamed']);
ignoreLogs([]);

const unmountFunctions: Array<() => void | boolean> = [];
Expand Down
8 changes: 4 additions & 4 deletions packages/react-dom-bindings/src/client/validateDOMNesting.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ function validateDOMNesting(
// TODO: Format this as a linkified "diff view" with props instead of
// a stack trace since the stack trace format is now for owner stacks.
console['error'](
'Warning: In HTML, %s cannot be a child of <%s>.%s\n' +
'In HTML, %s cannot be a child of <%s>.%s\n' +
'This will cause a hydration error.%s',
tagDisplayName,
ancestorTag,
Expand All @@ -498,7 +498,7 @@ function validateDOMNesting(
// TODO: Format this as a linkified "diff view" with props instead of
// a stack trace since the stack trace format is now for owner stacks.
console['error'](
'Warning: In HTML, %s cannot be a descendant of <%s>.\n' +
'In HTML, %s cannot be a descendant of <%s>.\n' +
'This will cause a hydration error.%s',
tagDisplayName,
ancestorTag,
Expand Down Expand Up @@ -530,7 +530,7 @@ function validateTextNesting(childText: string, parentTag: string): boolean {
// TODO: Format this as a linkified "diff view" with props instead of
// a stack trace since the stack trace format is now for owner stacks.
console['error'](
'Warning: In HTML, text nodes cannot be a child of <%s>.\n' +
'In HTML, text nodes cannot be a child of <%s>.\n' +
'This will cause a hydration error.%s',
parentTag,
getCurrentParentStackInDev(),
Expand All @@ -542,7 +542,7 @@ function validateTextNesting(childText: string, parentTag: string): boolean {
// TODO: Format this as a linkified "diff view" with props instead of
// a stack trace since the stack trace format is now for owner stacks.
console['error'](
'Warning: In HTML, whitespace text nodes cannot be a child of <%s>. ' +
'In HTML, whitespace text nodes cannot be a child of <%s>. ' +
"Make sure you don't have any extra whitespace between tags on " +
'each line of your source code.\n' +
'This will cause a hydration error.%s',
Expand Down
18 changes: 9 additions & 9 deletions packages/react-dom/src/__tests__/CSSPropertyOperations-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ describe('CSSPropertyOperations', () => {
root.render(<Comp />);
});
}).toErrorDev(
'Warning: Unsupported style property background-color. Did you mean backgroundColor?' +
'Unsupported style property background-color. Did you mean backgroundColor?' +
'\n in div (at **)' +
'\n in Comp (at **)',
);
Expand Down Expand Up @@ -137,10 +137,10 @@ describe('CSSPropertyOperations', () => {
root.render(<Comp style={styles} />);
});
}).toErrorDev([
'Warning: Unsupported style property -ms-transform. Did you mean msTransform?' +
'Unsupported style property -ms-transform. Did you mean msTransform?' +
'\n in div (at **)' +
'\n in Comp (at **)',
'Warning: Unsupported style property -webkit-transform. Did you mean WebkitTransform?' +
'Unsupported style property -webkit-transform. Did you mean WebkitTransform?' +
'\n in div (at **)' +
'\n in Comp (at **)',
]);
Expand Down Expand Up @@ -171,11 +171,11 @@ describe('CSSPropertyOperations', () => {
});
}).toErrorDev([
// msTransform is correct already and shouldn't warn
'Warning: Unsupported vendor-prefixed style property oTransform. ' +
'Unsupported vendor-prefixed style property oTransform. ' +
'Did you mean OTransform?' +
'\n in div (at **)' +
'\n in Comp (at **)',
'Warning: Unsupported vendor-prefixed style property webkitTransform. ' +
'Unsupported vendor-prefixed style property webkitTransform. ' +
'Did you mean WebkitTransform?' +
'\n in div (at **)' +
'\n in Comp (at **)',
Expand Down Expand Up @@ -207,11 +207,11 @@ describe('CSSPropertyOperations', () => {
root.render(<Comp />);
});
}).toErrorDev([
"Warning: Style property values shouldn't contain a semicolon. " +
"Style property values shouldn't contain a semicolon. " +
'Try "backgroundColor: blue" instead.' +
'\n in div (at **)' +
'\n in Comp (at **)',
"Warning: Style property values shouldn't contain a semicolon. " +
"Style property values shouldn't contain a semicolon. " +
'Try "color: red" instead.' +
'\n in div (at **)' +
'\n in Comp (at **)',
Expand All @@ -234,7 +234,7 @@ describe('CSSPropertyOperations', () => {
root.render(<Comp />);
});
}).toErrorDev(
'Warning: `NaN` is an invalid value for the `fontSize` css style property.' +
'`NaN` is an invalid value for the `fontSize` css style property.' +
'\n in div (at **)' +
'\n in Comp (at **)',
);
Expand Down Expand Up @@ -270,7 +270,7 @@ describe('CSSPropertyOperations', () => {
root.render(<Comp />);
});
}).toErrorDev(
'Warning: `Infinity` is an invalid value for the `fontSize` css style property.' +
'`Infinity` is an invalid value for the `fontSize` css style property.' +
'\n in div (at **)' +
'\n in Comp (at **)',
);
Expand Down
2 changes: 1 addition & 1 deletion packages/react-dom/src/__tests__/DOMPropertyOperations-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1333,7 +1333,7 @@ describe('DOMPropertyOperations', () => {
});

assertConsoleErrorDev([
'The `popoverTarget` prop expects the ID of an Element as a string. Received [object HTMLDivElement] instead.',
'The `popoverTarget` prop expects the ID of an Element as a string. Received HTMLDivElement {} instead.',
]);

// Dedupe warning
Expand Down
12 changes: 6 additions & 6 deletions packages/react-dom/src/__tests__/ReactComponent-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ describe('ReactComponent', () => {
root.render(<Component />);
});
}).toErrorDev([
'Warning: Component "Component" contains the string ref "inner". ' +
'Component "Component" contains the string ref "inner". ' +
'Support for string refs will be removed in a future major release. ' +
'We recommend using useRef() or createRef() instead. ' +
'Learn more about using refs safely here: https://react.dev/link/strict-mode-string-ref\n' +
Expand Down Expand Up @@ -711,7 +711,7 @@ describe('ReactComponent', () => {
root.render(<Foo />);
});
}).toErrorDev(
'Warning: Functions are not valid as a React child. This may happen if ' +
'Functions are not valid as a React child. This may happen if ' +
'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' +
Expand All @@ -733,7 +733,7 @@ describe('ReactComponent', () => {
root.render(<Foo />);
});
}).toErrorDev(
'Warning: Functions are not valid as a React child. This may happen if ' +
'Functions are not valid as a React child. This may happen if ' +
'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' +
Expand All @@ -756,7 +756,7 @@ describe('ReactComponent', () => {
root.render(<Foo />);
});
}).toErrorDev(
'Warning: Functions are not valid as a React child. This may happen if ' +
'Functions are not valid as a React child. This may happen if ' +
'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' +
Expand Down Expand Up @@ -811,13 +811,13 @@ describe('ReactComponent', () => {
root.render(<Foo ref={current => (component = current)} />);
});
}).toErrorDev([
'Warning: Functions are not valid as a React child. This may happen if ' +
'Functions are not valid as a React child. This may happen if ' +
'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 ' +
'Functions are not valid as a React child. This may happen if ' +
'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' +
Expand Down
8 changes: 4 additions & 4 deletions packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ describe('ReactComponentLifeCycle', () => {
root.render(<StatefulComponent />);
});
}).toErrorDev(
"Warning: Can't call setState on a component that is not yet mounted. " +
"Can't call setState on a component that is not yet mounted. " +
'This is a no-op, but it might indicate a bug in your application. ' +
'Instead, assign to `this.state` directly or define a `state = {};` ' +
'class property with the desired state in the StatefulComponent component.',
Expand Down Expand Up @@ -1504,20 +1504,20 @@ describe('ReactComponentLifeCycle', () => {
}).toWarnDev(
[
/* eslint-disable max-len */
`Warning: componentWillMount has been renamed, and is not recommended for use. See https://react.dev/link/unsafe-component-lifecycles for details.
`componentWillMount has been renamed, and is not recommended for use. See https://react.dev/link/unsafe-component-lifecycles for details.

* Move code with side effects to componentDidMount, and set initial state in the constructor.
* Rename componentWillMount to UNSAFE_componentWillMount to suppress this warning in non-strict mode. In React 18.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, you can run \`npx react-codemod rename-unsafe-lifecycles\` in your project source folder.

Please update the following components: MyComponent`,
`Warning: componentWillReceiveProps has been renamed, and is not recommended for use. See https://react.dev/link/unsafe-component-lifecycles for details.
`componentWillReceiveProps has been renamed, and is not recommended for use. See https://react.dev/link/unsafe-component-lifecycles for details.

* Move data fetching code or side effects to componentDidUpdate.
* If you're updating state whenever props change, refactor your code to use memoization techniques or move it to static getDerivedStateFromProps. Learn more at: https://react.dev/link/derived-state
* Rename componentWillReceiveProps to UNSAFE_componentWillReceiveProps to suppress this warning in non-strict mode. In React 18.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, you can run \`npx react-codemod rename-unsafe-lifecycles\` in your project source folder.

Please update the following components: MyComponent`,
`Warning: componentWillUpdate has been renamed, and is not recommended for use. See https://react.dev/link/unsafe-component-lifecycles for details.
`componentWillUpdate has been renamed, and is not recommended for use. See https://react.dev/link/unsafe-component-lifecycles for details.

* Move data fetching code or side effects to componentDidUpdate.
* Rename componentWillUpdate to UNSAFE_componentWillUpdate to suppress this warning in non-strict mode. In React 18.x, only the UNSAFE_ name will work. To rename all deprecated lifecycles to their new names, you can run \`npx react-codemod rename-unsafe-lifecycles\` in your project source folder.
Expand Down
22 changes: 11 additions & 11 deletions packages/react-dom/src/__tests__/ReactCompositeComponent-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ describe('ReactCompositeComponent', () => {
root.render(<MyComponent />);
});
}).toErrorDev(
"Warning: Can't call forceUpdate on a component that is not yet mounted. " +
"Can't call forceUpdate on a component that is not yet mounted. " +
'This is a no-op, but it might indicate a bug in your application. ' +
'Instead, assign to `this.state` directly or define a `state = {};` ' +
'class property with the desired state in the MyComponent component.',
Expand Down Expand Up @@ -347,7 +347,7 @@ describe('ReactCompositeComponent', () => {
root.render(<MyComponent />);
});
}).toErrorDev(
"Warning: Can't call setState on a component that is not yet mounted. " +
"Can't call setState on a component that is not yet mounted. " +
'This is a no-op, but it might indicate a bug in your application. ' +
'Instead, assign to `this.state` directly or define a `state = {};` ' +
'class property with the desired state in the MyComponent component.',
Expand Down Expand Up @@ -484,7 +484,7 @@ describe('ReactCompositeComponent', () => {
});
}).rejects.toThrow(TypeError);
}).toErrorDev(
'Warning: The <ClassWithRenderNotExtended /> component appears to have a render method, ' +
'The <ClassWithRenderNotExtended /> component appears to have a render method, ' +
"but doesn't extend React.Component. This is likely to cause errors. " +
'Change ClassWithRenderNotExtended to extend React.Component instead.',
);
Expand Down Expand Up @@ -631,7 +631,7 @@ describe('ReactCompositeComponent', () => {
instance.setState({bogus: true});
});
}).toErrorDev(
'Warning: ClassComponent.shouldComponentUpdate(): Returned undefined instead of a ' +
'ClassComponent.shouldComponentUpdate(): Returned undefined instead of a ' +
'boolean value. Make sure to return true or false.',
);
});
Expand All @@ -651,7 +651,7 @@ describe('ReactCompositeComponent', () => {
root.render(<Component />);
});
}).toErrorDev(
'Warning: Component has a method called ' +
'Component has a method called ' +
'componentDidUnmount(). But there is no such lifecycle method. ' +
'Did you mean componentWillUnmount()?',
);
Expand All @@ -673,7 +673,7 @@ describe('ReactCompositeComponent', () => {
root.render(<Component />);
});
}).toErrorDev(
'Warning: Component has a method called ' +
'Component has a method called ' +
'componentDidReceiveProps(). But there is no such lifecycle method. ' +
'If you meant to update the state in response to changing props, ' +
'use componentWillReceiveProps(). If you meant to fetch data or ' +
Expand All @@ -699,7 +699,7 @@ describe('ReactCompositeComponent', () => {
root.render(<Component />);
});
}).toErrorDev(
'Warning: Setting defaultProps as an instance property on Component is not supported ' +
'Setting defaultProps as an instance property on Component is not supported ' +
'and will be ignored. Instead, define defaultProps as a static property on Component.',
);
});
Expand Down Expand Up @@ -1199,9 +1199,9 @@ describe('ReactCompositeComponent', () => {
});
}).rejects.toThrow();
}).toErrorDev([
'Warning: No `render` method found on the RenderTextInvalidConstructor instance: ' +
'No `render` method found on the RenderTextInvalidConstructor instance: ' +
'did you accidentally return an object from the constructor?',
'Warning: No `render` method found on the RenderTextInvalidConstructor instance: ' +
'No `render` method found on the RenderTextInvalidConstructor instance: ' +
'did you accidentally return an object from the constructor?',
]);
});
Expand Down Expand Up @@ -1239,9 +1239,9 @@ describe('ReactCompositeComponent', () => {
});
}).rejects.toThrow();
}).toErrorDev([
'Warning: No `render` method found on the RenderTestUndefinedRender instance: ' +
'No `render` method found on the RenderTestUndefinedRender instance: ' +
'you may have forgotten to define `render`.',
'Warning: No `render` method found on the RenderTestUndefinedRender instance: ' +
'No `render` method found on the RenderTestUndefinedRender instance: ' +
'you may have forgotten to define `render`.',
]);
});
Expand Down
Loading