Skip to content

Commit a724a3b

Browse files
authored
[RFC] Codemod invariant -> throw new Error (#22435)
* Hoist error codes import to module scope When this code was written, the error codes map (`codes.json`) was created on-the-fly, so we had to lazily require from inside the visitor. Because `codes.json` is now checked into source, we can import it a single time in module scope. * Minify error constructors in production We use a script to minify our error messages in production. Each message is assigned an error code, defined in `scripts/error-codes/codes.json`. Then our build script replaces the messages with a link to our error decoder page, e.g. https://reactjs.org/docs/error-decoder.html/?invariant=92 This enables us to write helpful error messages without increasing the bundle size. Right now, the script only works for `invariant` calls. It does not work if you throw an Error object. This is an old Facebookism that we don't really need, other than the fact that our error minification script relies on it. So, I've updated the script to minify error constructors, too: Input: Error(`A ${adj} message that contains ${noun}`); Output: Error(formatProdErrorMessage(ERR_CODE, adj, noun)); It only works for constructors that are literally named Error, though we could add support for other names, too. As a next step, I will add a lint rule to enforce that errors written this way must have a corresponding error code. * Minify "no fallback UI specified" error in prod This error message wasn't being minified because it doesn't use invariant. The reason it didn't use invariant is because this particular error is created without begin thrown — it doesn't need to be thrown because it's located inside the error handling part of the runtime. Now that the error minification script supports Error constructors, we can minify it by assigning it a production error code in `scripts/error-codes/codes.json`. To support the use of Error constructors more generally, I will add a lint rule that enforces each message has a corresponding error code. * Lint rule to detect unminified errors Adds a lint rule that detects when an Error constructor is used without a corresponding production error code. We already have this for `invariant`, but not for regular errors, i.e. `throw new Error(msg)`. There's also nothing that enforces the use of `invariant` besides convention. There are some packages where we don't care to minify errors. These are packages that run in environments where bundle size is not a concern, like react-pg. I added an override in the ESLint config to ignore these. * Temporarily add invariant codemod script I'm adding this codemod to the repo temporarily, but I'll revert it in the same PR. That way we don't have to check it in but it's still accessible (via the PR) if we need it later. * [Automated] Codemod invariant -> Error This commit contains only automated changes: npx jscodeshift -t scripts/codemod-invariant.js packages --ignore-pattern="node_modules/**/*" yarn linc --fix yarn prettier I will do any manual touch ups in separate commits so they're easier to review. * Remove temporary codemod script This reverts the codemod script and ESLint config I added temporarily in order to perform the invariant codemod. * Manual touch ups A few manual changes I made after the codemod ran. * Enable error code transform per package Currently we're not consistent about which packages should have their errors minified in production and which ones should. This adds a field to the bundle configuration to control whether to apply the transform. We should decide what the criteria is going forward. I think it's probably a good idea to minify any package that gets sent over the network. So yes to modules that run in the browser, and no to modules that run on the server and during development only.
1 parent f5d946d commit a724a3b

File tree

128 files changed

+2118
-1519
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

128 files changed

+2118
-1519
lines changed

.eslintrc.js

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,41 @@ module.exports = {
127127
},
128128

129129
overrides: [
130+
{
131+
// By default, anything error message that appears the packages directory
132+
// must have a corresponding error code. The exceptions are defined
133+
// in the next override entry.
134+
files: ['packages/**/*.js'],
135+
rules: {
136+
'react-internal/prod-error-codes': ERROR,
137+
},
138+
},
139+
{
140+
// These are files where it's OK to have unminified error messages. These
141+
// are environments where bundle size isn't a concern, like tests
142+
// or Node.
143+
files: [
144+
'packages/react-dom/src/test-utils/**/*.js',
145+
'packages/react-devtools-shared/**/*.js',
146+
'packages/react-noop-renderer/**/*.js',
147+
'packages/react-pg/**/*.js',
148+
'packages/react-fs/**/*.js',
149+
'packages/react-refresh/**/*.js',
150+
'packages/react-server-dom-webpack/**/*.js',
151+
'packages/react-test-renderer/**/*.js',
152+
'packages/react-debug-tools/**/*.js',
153+
'packages/react-devtools-extensions/**/*.js',
154+
'packages/react-devtools-scheduling-profiler/**/*.js',
155+
'packages/react-native-renderer/**/*.js',
156+
'packages/eslint-plugin-react-hooks/**/*.js',
157+
'packages/jest-react/**/*.js',
158+
'packages/**/__tests__/*.js',
159+
'packages/**/npm/*.js',
160+
],
161+
rules: {
162+
'react-internal/prod-error-codes': OFF,
163+
},
164+
},
130165
{
131166
// We apply these settings to files that we ship through npm.
132167
// They must be ES5.

packages/create-subscription/src/createSubscription.js

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
*/
99

1010
import * as React from 'react';
11-
import invariant from 'shared/invariant';
1211

1312
type Unsubscribe = () => void;
1413

@@ -128,10 +127,12 @@ export function createSubscription<Property, Value>(
128127

129128
// Store the unsubscribe method for later (in case the subscribable prop changes).
130129
const unsubscribe = subscribe(source, callback);
131-
invariant(
132-
typeof unsubscribe === 'function',
133-
'A subscription must return an unsubscribe function.',
134-
);
130+
131+
if (typeof unsubscribe !== 'function') {
132+
throw new Error(
133+
'A subscription must return an unsubscribe function.',
134+
);
135+
}
135136

136137
// It's safe to store unsubscribe on the instance because
137138
// We only read or write that property during the "commit" phase.

packages/jest-react/src/JestReact.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
import {REACT_ELEMENT_TYPE, REACT_FRAGMENT_TYPE} from 'shared/ReactSymbols';
99

10-
import invariant from 'shared/invariant';
1110
import isArray from 'shared/isArray';
1211

1312
export {act} from './internalAct';
@@ -31,11 +30,12 @@ function captureAssertion(fn) {
3130
function assertYieldsWereCleared(root) {
3231
const Scheduler = root._Scheduler;
3332
const actualYields = Scheduler.unstable_clearYields();
34-
invariant(
35-
actualYields.length === 0,
36-
'Log of yielded values is not empty. ' +
37-
'Call expect(ReactTestRenderer).unstable_toHaveYielded(...) first.',
38-
);
33+
if (actualYields.length !== 0) {
34+
throw new Error(
35+
'Log of yielded values is not empty. ' +
36+
'Call expect(ReactTestRenderer).unstable_toHaveYielded(...) first.',
37+
);
38+
}
3939
}
4040

4141
export function unstable_toMatchRenderedOutput(root, expectedJSX) {

packages/react-art/src/ReactARTHostConfig.js

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
import Transform from 'art/core/transform';
99
import Mode from 'art/modes/current';
10-
import invariant from 'shared/invariant';
1110

1211
import {TYPES, EVENT_TYPES, childrenAsString} from './ReactARTInternals';
1312

@@ -248,8 +247,7 @@ export * from 'react-reconciler/src/ReactFiberHostConfigWithNoMicrotasks';
248247
export function appendInitialChild(parentInstance, child) {
249248
if (typeof child === 'string') {
250249
// Noop for string children of Text (eg <Text>{'foo'}{'bar'}</Text>)
251-
invariant(false, 'Text children should already be flattened.');
252-
return;
250+
throw new Error('Text children should already be flattened.');
253251
}
254252

255253
child.inject(parentInstance);
@@ -282,7 +280,9 @@ export function createInstance(type, props, internalInstanceHandle) {
282280
break;
283281
}
284282

285-
invariant(instance, 'ReactART does not support the type "%s"', type);
283+
if (!instance) {
284+
throw new Error(`ReactART does not support the type "${type}"`);
285+
}
286286

287287
instance._applyProps(instance, props);
288288

@@ -367,18 +367,18 @@ export function appendChildToContainer(parentInstance, child) {
367367
}
368368

369369
export function insertBefore(parentInstance, child, beforeChild) {
370-
invariant(
371-
child !== beforeChild,
372-
'ReactART: Can not insert node before itself',
373-
);
370+
if (child === beforeChild) {
371+
throw new Error('ReactART: Can not insert node before itself');
372+
}
373+
374374
child.injectBefore(beforeChild);
375375
}
376376

377377
export function insertInContainerBefore(parentInstance, child, beforeChild) {
378-
invariant(
379-
child !== beforeChild,
380-
'ReactART: Can not insert node before itself',
381-
);
378+
if (child === beforeChild) {
379+
throw new Error('ReactART: Can not insert node before itself');
380+
}
381+
382382
child.injectBefore(beforeChild);
383383
}
384384

@@ -433,25 +433,25 @@ export function clearContainer(container) {
433433
}
434434

435435
export function getInstanceFromNode(node) {
436-
throw new Error('Not yet implemented.');
436+
throw new Error('Not implemented.');
437437
}
438438

439439
export function isOpaqueHydratingObject(value: mixed): boolean {
440-
throw new Error('Not yet implemented');
440+
throw new Error('Not implemented.');
441441
}
442442

443443
export function makeOpaqueHydratingObject(
444444
attemptToReadValue: () => void,
445445
): OpaqueIDType {
446-
throw new Error('Not yet implemented.');
446+
throw new Error('Not implemented.');
447447
}
448448

449449
export function makeClientId(): OpaqueIDType {
450-
throw new Error('Not yet implemented');
450+
throw new Error('Not implemented.');
451451
}
452452

453453
export function makeClientIdInDEV(warnOnAccessInDEV: () => void): OpaqueIDType {
454-
throw new Error('Not yet implemented');
454+
throw new Error('Not implemented.');
455455
}
456456

457457
export function beforeActiveInstanceBlur(internalInstanceHandle: Object) {

packages/react-cache/src/ReactCacheOld.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ const ReactCurrentDispatcher =
4949
function readContext(Context) {
5050
const dispatcher = ReactCurrentDispatcher.current;
5151
if (dispatcher === null) {
52+
// This wasn't being minified but we're going to retire this package anyway.
53+
// eslint-disable-next-line react-internal/prod-error-codes
5254
throw new Error(
5355
'react-cache: read and preload may only be called from within a ' +
5456
"component's render. They are not supported in event handlers or " +

packages/react-client/src/ReactFlightClient.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,7 @@ export function resolveError(
397397
message: string,
398398
stack: string,
399399
): void {
400+
// eslint-disable-next-line react-internal/prod-error-codes
400401
const error = new Error(message);
401402
error.stack = stack;
402403
const chunks = response._chunks;

packages/react-client/src/ReactFlightClientHostConfig.js

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@
77
* @flow
88
*/
99

10-
/* eslint-disable react-internal/invariant-args */
11-
12-
import invariant from 'shared/invariant';
10+
/* eslint-disable react-internal/prod-error-codes */
1311

1412
// We expect that our Rollup, Jest, and Flow configurations
1513
// always shim this module with the corresponding host config
@@ -19,4 +17,4 @@ import invariant from 'shared/invariant';
1917
// sure that if we *do* accidentally break the configuration,
2018
// the failure isn't silent.
2119

22-
invariant(false, 'This module must be shimmed by a specific renderer.');
20+
throw new Error('This module must be shimmed by a specific renderer.');

packages/react-client/src/ReactFlightClientHostConfigNoStream.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,22 @@ export type StringDecoder = void;
1212
export const supportsBinaryStreams = false;
1313

1414
export function createStringDecoder(): void {
15+
// eslint-disable-next-line react-internal/prod-error-codes
1516
throw new Error('Should never be called');
1617
}
1718

1819
export function readPartialStringChunk(
1920
decoder: StringDecoder,
2021
buffer: Uint8Array,
2122
): string {
23+
// eslint-disable-next-line react-internal/prod-error-codes
2224
throw new Error('Should never be called');
2325
}
2426

2527
export function readFinalStringChunk(
2628
decoder: StringDecoder,
2729
buffer: Uint8Array,
2830
): string {
31+
// eslint-disable-next-line react-internal/prod-error-codes
2932
throw new Error('Should never be called');
3033
}

packages/react-debug-tools/src/ReactDebugHooks.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import type {OpaqueIDType} from 'react-reconciler/src/ReactFiberHostConfig';
2323
import {NoMode} from 'react-reconciler/src/ReactTypeOfMode';
2424

2525
import ErrorStackParser from 'error-stack-parser';
26-
import invariant from 'shared/invariant';
2726
import ReactSharedInternals from 'shared/ReactSharedInternals';
2827
import {REACT_OPAQUE_ID_TYPE} from 'shared/ReactSymbols';
2928
import {
@@ -107,7 +106,7 @@ function nextHook(): null | Hook {
107106
}
108107

109108
function getCacheForType<T>(resourceType: () => T): T {
110-
invariant(false, 'Not implemented.');
109+
throw new Error('Not implemented.');
111110
}
112111

113112
function readContext<T>(context: ReactContext<T>): T {

packages/react-dom/src/client/ReactDOM.js

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ import {
3939
import {createPortal as createPortalImpl} from 'react-reconciler/src/ReactPortal';
4040
import {canUseDOM} from 'shared/ExecutionEnvironment';
4141
import ReactVersion from 'shared/ReactVersion';
42-
import invariant from 'shared/invariant';
4342
import {
4443
warnUnstableRenderSubtreeIntoContainer,
4544
enableNewReconciler,
@@ -108,10 +107,10 @@ function createPortal(
108107
container: Container,
109108
key: ?string = null,
110109
): React$Portal {
111-
invariant(
112-
isValidContainer(container),
113-
'Target container is not a DOM element.',
114-
);
110+
if (!isValidContainer(container)) {
111+
throw new Error('Target container is not a DOM element.');
112+
}
113+
115114
// TODO: pass ReactDOM portal implementation as third argument
116115
// $FlowFixMe The Flow type is opaque but there's no way to actually create it.
117116
return createPortalImpl(children, container, null, key);

packages/react-dom/src/client/ReactDOMComponentTree.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import {
3030

3131
import {getParentSuspenseInstance} from './ReactDOMHostConfig';
3232

33-
import invariant from 'shared/invariant';
3433
import {enableScopeAPI} from 'shared/ReactFeatureFlags';
3534

3635
const randomKey = Math.random()
@@ -190,7 +189,7 @@ export function getNodeFromInstance(inst: Fiber): Instance | TextInstance {
190189

191190
// Without this first invariant, passing a non-DOM-component triggers the next
192191
// invariant for a missing parent, which is super confusing.
193-
invariant(false, 'getNodeFromInstance: Invalid argument.');
192+
throw new Error('getNodeFromInstance: Invalid argument.');
194193
}
195194

196195
export function getFiberCurrentPropsFromNode(

packages/react-dom/src/client/ReactDOMEventHandle.js

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import {
2828
enableScopeAPI,
2929
enableCreateEventHandleAPI,
3030
} from 'shared/ReactFeatureFlags';
31-
import invariant from 'shared/invariant';
3231

3332
type EventHandleOptions = {|
3433
capture?: boolean,
@@ -73,8 +72,7 @@ function registerReactDOMEvent(
7372
eventTarget,
7473
);
7574
} else {
76-
invariant(
77-
false,
75+
throw new Error(
7876
'ReactDOM.createEventHandle: setter called on an invalid ' +
7977
'target. Provide a valid EventTarget or an element managed by React.',
8078
);
@@ -97,11 +95,11 @@ export function createEventHandle(
9795
// Unfortunately, the downside of this invariant is that *removing* a native
9896
// event from the list of known events has now become a breaking change for
9997
// any code relying on the createEventHandle API.
100-
invariant(
101-
allNativeEvents.has(domEventName),
102-
'Cannot call unstable_createEventHandle with "%s", as it is not an event known to React.',
103-
domEventName,
104-
);
98+
if (!allNativeEvents.has(domEventName)) {
99+
throw new Error(
100+
`Cannot call unstable_createEventHandle with "${domEventName}", as it is not an event known to React.`,
101+
);
102+
}
105103

106104
let isCapturePhaseListener = false;
107105
if (options != null) {
@@ -115,11 +113,13 @@ export function createEventHandle(
115113
target: EventTarget | ReactScopeInstance,
116114
callback: (SyntheticEvent<EventTarget>) => void,
117115
) => {
118-
invariant(
119-
typeof callback === 'function',
120-
'ReactDOM.createEventHandle: setter called with an invalid ' +
121-
'callback. The callback must be a function.',
122-
);
116+
if (typeof callback !== 'function') {
117+
throw new Error(
118+
'ReactDOM.createEventHandle: setter called with an invalid ' +
119+
'callback. The callback must be a function.',
120+
);
121+
}
122+
123123
if (!doesTargetHaveEventHandle(target, eventHandle)) {
124124
addEventHandleToTarget(target, eventHandle);
125125
registerReactDOMEvent(target, domEventName, isCapturePhaseListener);

packages/react-dom/src/client/ReactDOMInput.js

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
// TODO: direct imports like some-package/src/* are bad. Fix me.
1111
import {getCurrentFiberOwnerNameInDevOrNull} from 'react-reconciler/src/ReactCurrentFiber';
12-
import invariant from 'shared/invariant';
1312

1413
import {setValueForProperty} from './DOMPropertyOperations';
1514
import {getFiberCurrentPropsFromNode} from './ReactDOMComponentTree';
@@ -383,11 +382,13 @@ function updateNamedCousins(rootNode, props) {
383382
// That's probably okay; we don't support it just as we don't support
384383
// mixing React radio buttons with non-React ones.
385384
const otherProps = getFiberCurrentPropsFromNode(otherNode);
386-
invariant(
387-
otherProps,
388-
'ReactDOMInput: Mixing React and non-React radio inputs with the ' +
389-
'same `name` is not supported.',
390-
);
385+
386+
if (!otherProps) {
387+
throw new Error(
388+
'ReactDOMInput: Mixing React and non-React radio inputs with the ' +
389+
'same `name` is not supported.',
390+
);
391+
}
391392

392393
// We need update the tracked value on the named cousin since the value
393394
// was changed but the input saw no event or value set

0 commit comments

Comments
 (0)