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

Lint rule for unminified errors #15757

Merged
merged 2 commits into from
May 29, 2019
Merged
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
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
"linc": "node ./scripts/tasks/linc.js",
"lint": "node ./scripts/tasks/eslint.js",
"lint-build": "node ./scripts/rollup/validate/index.js",
"extract-errors": "yarn build --type=dev --extract-errors",
"postinstall": "node node_modules/fbjs-scripts/node/check-dev-engines.js package.json && node ./scripts/flow/createFlowConfigs.js",
"debug-test": "cross-env NODE_ENV=development node --inspect-brk node_modules/.bin/jest --config ./scripts/jest/config.source.js --runInBand",
"test": "cross-env NODE_ENV=development jest --config ./scripts/jest/config.source.js",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
* LICENSE file in the root directory of this source tree.
*/

/* eslint-disable react-internal/warning-and-invariant-args */

'use strict';

// Mock of the Native Hooks
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
* @flow strict-local
*/

/* eslint-disable react-internal/warning-and-invariant-args */

'use strict';

import type {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
* LICENSE file in the root directory of this source tree.
*/

/* eslint-disable react-internal/warning-and-invariant-args */

'use strict';

// Mock of the Native Hooks
Expand Down
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactFiberHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
* @flow
*/

/* eslint-disable react-internal/warning-and-invariant-args */

import invariant from 'shared/invariant';

// We expect that our Rollup, Jest, and Flow configurations
Expand Down
2 changes: 2 additions & 0 deletions packages/react-stream/src/ReactFizzFormatConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
* @flow
*/

/* eslint-disable react-internal/warning-and-invariant-args */

import invariant from 'shared/invariant';

// We expect that our Rollup, Jest, and Flow configurations
Expand Down
2 changes: 2 additions & 0 deletions packages/react-stream/src/ReactFizzHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
* @flow
*/

/* eslint-disable react-internal/warning-and-invariant-args */

import invariant from 'shared/invariant';

// We expect that our Rollup, Jest, and Flow configurations
Expand Down
4 changes: 1 addition & 3 deletions scripts/error-codes/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ provide a better debugging support in production. Check out the blog post
the file will never be changed/removed.
- [`extract-errors.js`](https://github.com/facebook/react/blob/master/scripts/error-codes/extract-errors.js)
is an node script that traverses our codebase and updates `codes.json`. You
can test it by running `yarn build -- --extract-errors`, but you should only
commit changes to this file when running a release. (The release tool will
perform this step automatically.)
can test it by running `yarn extract-errors`.
- [`transform-error-messages`](https://github.com/facebook/react/blob/master/scripts/error-codes/transform-error-messages)
is a Babel pass that rewrites error messages to IDs for a production
(minified) build.
16 changes: 14 additions & 2 deletions scripts/error-codes/codes.json
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,6 @@
"220": "Container does not support insertBefore operation",
"221": "Tried to register two views with the same name %s",
"222": "View config not found for name %s",
"223": "Trying to release an event instance into a pool of a different type.",
"224": "Can't read from currently-mounting component. This error is likely caused by a bug in React. Please file an issue.",
"225": "Unexpected object passed to ReactTestInstance constructor (tag: %s). This is probably a bug in React.",
"226": "Unsupported component type %s in test renderer. This is probably a bug in React.",
Expand Down Expand Up @@ -321,5 +320,18 @@
"319": "A dehydrated suspense boundary must commit before trying to render. This is probably a bug in React.",
"320": "Expected ReactFiberErrorDialog.showErrorDialog to be a function.",
"321": "Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:\n1. You might have mismatching versions of React and the renderer (such as React DOM)\n2. You might be breaking the Rules of Hooks\n3. You might have more than one copy of React in the same app\nSee https://fb.me/react-invalid-hook-call for tips about how to debug and fix this problem.",
"322": "forwardRef requires a render function but was given %s."
"322": "forwardRef requires a render function but was given %s.",
"323": "React has blocked a javascript: URL as a security precaution.%s",
"324": "An event responder context was used outside of an event cycle. Use context.setTimeout() to use asynchronous responder context outside of event cycle .",
"325": "addRootEventTypes() found a duplicate root event type of \"%s\". This might be because the event type exists in the event responder \"rootEventTypes\" array or because of a previous addRootEventTypes() using this root event type.",
"326": "Expected a valid priority level",
"327": "Should not already be working.",
"328": "Should have a work-in-progress.",
"329": "Unknown root exit status.",
"330": "Should be working on an effect.",
"331": "Cannot flush passive effects while already rendering.",
"332": "Unknown priority level.",
"333": "This should have a parent host component initialized. This error is likely caused by a bug in React. Please file an issue.",
"334": "accumulate(...): Accumulated items must not be null or undefined.",
"335": "ReactDOMServer does not yet support the event API."
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ ruleTester.run('eslint-rules/warning-and-invariant-args', rule, {
valid: [
"warning(true, 'hello, world');",
"warning(true, 'expected %s, got %s', 42, 24);",
"invariant(true, 'hello, world');",
"invariant(true, 'expected %s, got %s', 42, 24);",
'arbitraryFunction(a, b)',
// These messages are in the error code map
"invariant(false, 'Do not override existing functions.')",
"invariant(false, '%s(...): Target container is not a DOM element.', str)",
],
invalid: [
{
Expand Down Expand Up @@ -96,5 +98,18 @@ ruleTester.run('eslint-rules/warning-and-invariant-args', rule, {
},
],
},
{
code: "invariant(false, 'Not in error map')",
errors: [
{
message:
'Error message does not have a corresponding production error code.\n\n' +
'Run `yarn extract-errors` to add the message to error code map, ' +
'so it can be stripped from the production builds. ' +
"Alternatively, if you're updating an existing error message, " +
'you can modify `scripts/error-codes/codes.json` directly.',
},
],
},
],
});
25 changes: 25 additions & 0 deletions scripts/eslint-rules/warning-and-invariant-args.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,16 @@

'use strict';

const fs = require('fs');
const path = require('path');
const existingErrorMap = JSON.parse(
fs.readFileSync(path.resolve(__dirname, '../error-codes/codes.json'))
);
const messages = new Set();
Object.keys(existingErrorMap).forEach(key =>
messages.add(existingErrorMap[key])
);

/**
* The warning() and invariant() functions take format strings as their second
* argument.
Expand Down Expand Up @@ -79,6 +89,21 @@ module.exports = function(context) {
}
);
}

if (node.callee.name === 'invariant') {
if (!messages.has(format)) {
context.report(
node,
'Error message does not have a corresponding production ' +
'error code.\n\n' +
'Run `yarn extract-errors` to add the message to error code ' +
'map, so it can be stripped from the production builds. ' +
"Alternatively, if you're updating an existing error " +
'message, you can modify ' +
'`scripts/error-codes/codes.json` directly.'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like it would be nice to log the error message too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured this was fine because if you have ESLint in your editor, it will display the lint error inline with the message.

);
}
}
},
};
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
* @flow strict-local
*/

/* eslint-disable react-internal/warning-and-invariant-args */

'use strict';

import type {
Expand Down