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.

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

[Outdated; needs revision] Add support for normal errors to error code infra #15072

Closed
wants to merge 5 commits into from
Closed
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
12 changes: 10 additions & 2 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ module.exports = {
'react-internal/no-primitive-constructors': ERROR,
'react-internal/no-to-warn-dev-within-to-throw': ERROR,
'react-internal/warning-and-invariant-args': ERROR,
'react-internal/static-error-messages': ERROR,
},

overrides: [
Expand Down Expand Up @@ -124,8 +125,15 @@ module.exports = {
rules: {
// https://github.com/jest-community/eslint-plugin-jest
'jest/no-focused-tests': ERROR,
}
}
},
},

{
files: ['scripts/**/*.js', '**/__tests__/*.js'],
rules: {
'react-internal/static-error-messages': OFF,
},
},
],

globals: {
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
Original file line number Diff line number Diff line change
Expand Up @@ -1301,6 +1301,7 @@ function toPropertyAccessString(node) {
const property = toPropertyAccessString(node.property);
return `${object}.${property}`;
} else {
// extract-errors/skip
throw new Error(`Unsupported node type: ${node.type}`);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ if (__DEV__) {
) {
return null;
}
// extract-errors/skip
return new Error(
'You provided a `value` prop to a form field without an ' +
'`onChange` handler. This will render a read-only field. If ' +
Expand All @@ -54,6 +55,7 @@ if (__DEV__) {
) {
return null;
}
// extract-errors/skip
return new Error(
'You provided a `checked` prop to a form field without an ' +
'`onChange` handler. This will render a read-only field. If ' +
Expand Down
3 changes: 3 additions & 0 deletions packages/react-dom/src/test-utils/ReactTestUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ const ReactTestUtils = {
className,
);
if (all.length !== 1) {
// extract-errors/skip
throw new Error(
'Did not find exactly one match (found: ' +
all.length +
Expand Down Expand Up @@ -305,6 +306,7 @@ const ReactTestUtils = {
validateClassInstance(root, 'findRenderedDOMComponentWithTag');
const all = ReactTestUtils.scryRenderedDOMComponentsWithTag(root, tagName);
if (all.length !== 1) {
// extract-errors/skip
throw new Error(
'Did not find exactly one match (found: ' +
all.length +
Expand Down Expand Up @@ -340,6 +342,7 @@ const ReactTestUtils = {
componentType,
);
if (all.length !== 1) {
// extract-errors/skip
throw new Error(
'Did not find exactly one match (found: ' +
all.length +
Expand Down
13 changes: 6 additions & 7 deletions packages/react-native-renderer/src/NativeMethodsMixinUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,18 @@ export function throwOnStylesProp(component: any, props: any) {
if (props.styles !== undefined) {
const owner = component._owner || null;
const name = component.constructor.displayName;
let msg =
'`styles` is not a supported property of `' +
name +
'`, did ' +
'you mean `style` (singular)?';
let addendum = '';
if (owner && owner.constructor && owner.constructor.displayName) {
msg +=
addendum +=
'\n\nCheck the `' +
owner.constructor.displayName +
'` parent ' +
' component.';
}
throw new Error(msg);
throw new Error(
`\`styles\` is not a supported property of \`${name}\`, did you ` +
`mean \`style\` (singular)?${addendum}`,
);
}
}

Expand Down
14 changes: 14 additions & 0 deletions packages/react-noop-renderer/src/createReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
if (typeof parentInstance.rootID !== 'string') {
// Some calls to this aren't typesafe.
// This helps surface mistakes in tests.
// extract-errors/skip
throw new Error(
'appendChildToContainer() first argument is not a container.',
);
Expand All @@ -127,6 +128,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
if (typeof (parentInstance: any).rootID === 'string') {
// Some calls to this aren't typesafe.
// This helps surface mistakes in tests.
// extract-errors/skip
throw new Error('appendChild() first argument is not an instance.');
}
appendChildToContainerOrInstance(parentInstance, child);
Expand All @@ -143,6 +145,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
}
const beforeIndex = parentInstance.children.indexOf(beforeChild);
if (beforeIndex === -1) {
// extract-errors/skip
throw new Error('This child does not exist.');
}
parentInstance.children.splice(beforeIndex, 0, child);
Expand All @@ -156,6 +159,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
if (typeof parentInstance.rootID !== 'string') {
// Some calls to this aren't typesafe.
// This helps surface mistakes in tests.
// extract-errors/skip
throw new Error(
'insertInContainerBefore() first argument is not a container.',
);
Expand All @@ -171,6 +175,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
if (typeof (parentInstance: any).rootID === 'string') {
// Some calls to this aren't typesafe.
// This helps surface mistakes in tests.
// extract-errors/skip
throw new Error('insertBefore() first argument is not an instance.');
}
insertInContainerOrInstanceBefore(parentInstance, child, beforeChild);
Expand All @@ -182,6 +187,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
): void {
const index = parentInstance.children.indexOf(child);
if (index === -1) {
// extract-errors/skip
throw new Error('This child does not exist.');
}
parentInstance.children.splice(index, 1);
Expand All @@ -194,6 +200,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
if (typeof parentInstance.rootID !== 'string') {
// Some calls to this aren't typesafe.
// This helps surface mistakes in tests.
// extract-errors/skip
throw new Error(
'removeChildFromContainer() first argument is not a container.',
);
Expand All @@ -208,6 +215,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
if (typeof (parentInstance: any).rootID === 'string') {
// Some calls to this aren't typesafe.
// This helps surface mistakes in tests.
// extract-errors/skip
throw new Error('removeChild() first argument is not an instance.');
}
removeChildFromContainerOrInstance(parentInstance, child);
Expand Down Expand Up @@ -252,6 +260,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {

function shouldSetTextContent(type: string, props: Props): boolean {
if (type === 'errorInBeginPhase') {
// extract-errors/skip
throw new Error('Error in host config.');
}
return (
Expand Down Expand Up @@ -320,6 +329,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
hostContext: HostContext,
): Instance {
if (type === 'errorInCompletePhase') {
// extract-errors/skip
throw new Error('Error in host config.');
}
const inst = {
Expand Down Expand Up @@ -368,12 +378,15 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
newProps: Props,
): null | {} {
if (type === 'errorInCompletePhase') {
// extract-errors/skip
throw new Error('Error in host config.');
}
if (oldProps === null) {
// extract-errors/skip
throw new Error('Should have old props');
}
if (newProps === null) {
// extract-errors/skip
throw new Error('Should have new props');
}
hostDiffCounter++;
Expand Down Expand Up @@ -510,6 +523,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
newProps: Props,
): void {
if (oldProps === null) {
// extract-errors/skip
throw new Error('Should have old props');
}
hostUpdateCounter++;
Expand Down
15 changes: 9 additions & 6 deletions packages/react-reconciler/src/ReactFiberUnwindWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -373,14 +373,17 @@ function throwException(
workInProgress = workInProgress.return;
} while (workInProgress !== null);
// No boundary was found. Fallthrough to error mode.
// TODO: Use invariant so the message is stripped in prod?

// TODO: The stack should be provided as a dev warning, but not an error, so
// it doesn't add to the production build.
const componentName =
getComponentName(sourceFiber.type) || 'A React component';
const componentStack = getStackByFiberInDevAndProd(sourceFiber);
value = new Error(
(getComponentName(sourceFiber.type) || 'A React component') +
' suspended while rendering, but no fallback UI was specified.\n' +
'\n' +
`${componentName} suspended while rendering, but no fallback UI ` +
'was specified.\n\n' +
'Add a <Suspense fallback=...> component higher in the tree to ' +
'provide a loading indicator or placeholder to display.' +
getStackByFiberInDevAndProd(sourceFiber),
`provide a loading indicator or placeholder to display.${componentStack}`,
);
}

Expand Down
5 changes: 5 additions & 0 deletions packages/react-test-renderer/src/ReactTestRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ function toJSON(inst: Instance | TextInstance): ReactTestRendererNode | null {
return json;
}
default:
// extract-errors/skip
throw new Error(`Unexpected node type in toJSON: ${inst.tag}`);
}
}
Expand Down Expand Up @@ -406,6 +407,7 @@ function expectOne(
? 'No instances found '
: `Expected 1 but found ${all.length} instances `;

// extract-errors/skip
throw new Error(prefix + message);
}

Expand Down Expand Up @@ -524,10 +526,12 @@ const ReactTestRendererFiber = {
enumerable: true,
get: function() {
if (root === null) {
// extract-errors/skip
throw new Error("Can't access .root on unmounted test renderer");
}
const children = getChildren(root.current);
if (children.length === 0) {
// extract-errors/skip
throw new Error("Can't access .root on unmounted test renderer");
} else if (children.length === 1) {
// Normally, we skip the root and just give you the child.
Expand Down Expand Up @@ -566,6 +570,7 @@ function wrapFiber(fiber: Fiber): ReactTestInstance {
// Enable ReactTestRenderer to be used to test DevTools integration.
injectIntoDevTools({
findFiberByHostInstance: (() => {
// extract-errors/skip
throw new Error('TestRenderer does not support findFiberByHostInstance()');
}: any),
bundleType: __DEV__ ? 1 : 0,
Expand Down
6 changes: 6 additions & 0 deletions packages/scheduler/src/forks/SchedulerHostConfig.mock.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export function getCurrentTime(): number {

export function reset() {
if (isFlushing) {
// extract-errors/skip
throw new Error('Cannot reset while already flushing work.');
}
currentTime = 0;
Expand All @@ -63,6 +64,7 @@ export function reset() {
// Should only be used via an assertion helper that inspects the yielded values.
export function unstable_flushNumberOfYields(count: number): void {
if (isFlushing) {
// extract-errors/skip
throw new Error('Already flushing work.');
}
expectedNumberOfYields = count;
Expand All @@ -85,6 +87,7 @@ export function unstable_flushNumberOfYields(count: number): void {

export function unstable_flushExpired() {
if (isFlushing) {
// extract-errors/skip
throw new Error('Already flushing work.');
}
if (scheduledCallback !== null) {
Expand All @@ -101,6 +104,7 @@ export function unstable_flushExpired() {

export function unstable_flushWithoutYielding(): void {
if (isFlushing) {
// extract-errors/skip
throw new Error('Already flushing work.');
}
isFlushing = true;
Expand Down Expand Up @@ -131,13 +135,15 @@ export function unstable_clearYields(): Array<mixed> {

export function flushAll(): void {
if (yieldedValues !== null) {
// extract-errors/skip
throw new Error(
'Log is not empty. Assert on the log of yielded values before ' +
'flushing additional work.',
);
}
unstable_flushWithoutYielding();
if (yieldedValues !== null) {
// extract-errors/skip
throw new Error(
'While flushing work, something yielded a value. Use an ' +
'assertion helper to assert on the log of yielded values, e.g. ' +
Expand Down
1 change: 1 addition & 0 deletions packages/shared/ReactError.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
// build, and in production they will be minified.

function ReactError(message) {
// extract-errors/skip
const error = new Error(message);
error.name = 'Invariant Violation';
return error;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/ReactErrorProd.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ function ReactErrorProd(code) {
for (let i = 1; i < arguments.length; i++) {
url += '&args[]=' + encodeURIComponent(arguments[i]);
}
// extract-errors/skip
return new Error(
`Minified React error #${code}; visit ${url} for the full message or ` +
'use the non-minified dev environment for full errors and additional ' +
Expand Down
4 changes: 1 addition & 3 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ export const enableSuspenseServerRenderer = false; // TODO: __DEV__? Here it mig
export const enableSchedulerDebugging = false;

// Only used in www builds.
export function addUserTimingListener() {
throw new Error('Not implemented.');
}
export function addUserTimingListener() {}

// Disable javascript: URL strings in href for XSS protection.
export const disableJavaScriptURLs = false;
Expand Down
1 change: 1 addition & 0 deletions packages/shared/invariant.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/

export default function invariant(condition, format, a, b, c, d, e, f) {
// extract-errors/skip
throw new Error(
'Internal React error: invariant() is meant to be replaced at compile ' +
'time. There is no runtime version.',
Expand Down
2 changes: 2 additions & 0 deletions packages/shared/invokeGuardedCallbackImpl.js
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ if (__DEV__) {
if (didError) {
if (!didSetError) {
// The callback errored, but the error event never fired.
// extract-errors/skip
error = new Error(
'An error was thrown inside one of your components, but React ' +
"doesn't know what it was. This is likely due to browser " +
Expand All @@ -197,6 +198,7 @@ if (__DEV__) {
'actually an issue with React, please file an issue.',
);
} else if (isCrossOriginError) {
// extract-errors/skip
error = new Error(
"A cross-origin error was thrown. React doesn't have access to " +
'the actual error object in development. ' +
Expand Down
2 changes: 2 additions & 0 deletions packages/shared/lowPriorityWarning.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,14 @@ if (__DEV__) {
// --- Welcome to debugging React ---
// This error was thrown as a convenience so that you can use this stack
// to find the callsite that caused this warning to fire.
// extract-errors/skip
throw new Error(message);
} catch (x) {}
};

lowPriorityWarning = function(condition, format, ...args) {
if (format === undefined) {
// extract-errors/skip
throw new Error(
'`lowPriorityWarning(condition, format, ...args)` requires a warning ' +
'message argument',
Expand Down
Loading