Skip to content

Commit

Permalink
Use comment to opt out of error extraction
Browse files Browse the repository at this point in the history
  • Loading branch information
acdlite committed Mar 9, 2019
1 parent 6ffefb0 commit d942f0e
Show file tree
Hide file tree
Showing 18 changed files with 97 additions and 25 deletions.
6 changes: 2 additions & 4 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +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,

// TODO: Enable this once we have a way to opt out of error extraction.
'react-internal/static-error-messages': OFF,
'react-internal/static-error-messages': ERROR,
},

overrides: [
Expand Down Expand Up @@ -131,7 +129,7 @@ module.exports = {
},

{
files: ['scripts/**/*.js'],
files: ['scripts/**/*.js', '**/__tests__/*.js'],
rules: {
'react-internal/static-error-messages': OFF,
},
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 @@ -1233,6 +1233,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 @@ -266,6 +266,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 @@ -302,6 +303,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 @@ -337,6 +339,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 @@ -75,6 +75,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 @@ -89,6 +90,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 @@ -105,6 +107,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 @@ -118,6 +121,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 @@ -133,6 +137,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 @@ -144,6 +149,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 @@ -156,6 +162,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 @@ -170,6 +177,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 @@ -209,6 +217,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 All @@ -231,6 +240,7 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {

createInstance(type: string, props: Props): Instance {
if (type === 'errorInCompletePhase') {
// extract-errors/skip
throw new Error('Error in host config.');
}
const inst = {
Expand Down Expand Up @@ -274,12 +284,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 @@ -337,6 +350,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 @@ -387,14 +387,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 @@ -116,6 +116,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 @@ -411,6 +412,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 @@ -529,10 +531,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 @@ -623,6 +627,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
2 changes: 1 addition & 1 deletion packages/shared/ReactError.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
// build, and in production they will be minified.

function ReactError(message) {
// eslint-disable-next-line react-internal/static-error-messages
// extract-errors/skip
const error = new Error(message);
error.name = 'Invariant Violation';
return error;
Expand Down
10 changes: 5 additions & 5 deletions packages/shared/ReactErrorProd.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ function ReactErrorProd(code, args) {
url += '&args[]=' + encodeURIComponent(args[i]);
}
}

const msg =
// 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 ' +
'helpful warnings. ';
return new Error(msg);
'use the non-minified dev environment for full errors and additional ' +
'helpful warnings. ',
);
}

export default ReactErrorProd;
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
3 changes: 2 additions & 1 deletion packages/shared/lowPriorityWarning.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +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.
// eslint-disable-next-line react-internal/static-error-messages
// 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
4 changes: 3 additions & 1 deletion packages/shared/warningWithoutStack.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@ let warningWithoutStack = () => {};
if (__DEV__) {
warningWithoutStack = function(condition, format, ...args) {
if (format === undefined) {
// extract-errors/skip
throw new Error(
'`warningWithoutStack(condition, format, ...args)` requires a warning ' +
'message argument',
);
}
if (args.length > 8) {
// Check before the condition to catch violations early.
// extract-errors/skip
throw new Error(
'warningWithoutStack() currently supports at most 8 arguments.',
);
Expand All @@ -46,7 +48,7 @@ if (__DEV__) {
let argIndex = 0;
const message =
'Warning: ' + format.replace(/%s/g, () => args[argIndex++]);
// eslint-disable-next-line react-internal/static-error-messages
// extract-errors/skip
throw new Error(message);
} catch (x) {}
};
Expand Down
Loading

0 comments on commit d942f0e

Please sign in to comment.