Skip to content

Commit

Permalink
Don't extract errors in CI
Browse files Browse the repository at this point in the history
Removes `--extract-errors` argument from CI build script command.
Instead, the author is expected to run `yarn extract-errors` locally
or manually edit the error code map.

The lint rule should be sufficient to catch unminified errors, but
as an extra precaution, I added a post-build step that greps the
production bundles. The post-build step works even if someone disables
the lint rule for a specific line or file.
  • Loading branch information
acdlite committed May 29, 2019
1 parent 112168f commit fc36310
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 20 deletions.
17 changes: 15 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ jobs:
- *run_yarn
- run: ./scripts/circleci/add_build_info_json.sh
- run: ./scripts/circleci/update_package_versions.sh
- run: ./scripts/circleci/build.sh
- run: yarn build
- run: cp ./scripts/rollup/results.json ./build/bundle-sizes.json
- run: ./scripts/circleci/upload_build.sh
- run: ./scripts/circleci/pack_and_store_artifact.sh
Expand All @@ -145,7 +145,6 @@ jobs:
- bundle-sizes.json

sizebot:
build:
docker: *docker
environment: *environment
steps:
Expand All @@ -155,6 +154,17 @@ jobs:
- *run_yarn
- run: node ./scripts/tasks/danger

lint_build:
docker: *docker
environment: *environment
steps:
- checkout
- attach_workspace: *attach_workspace
- *restore_yarn_cache
- *run_yarn
- run: yarn lint-build
- run: scripts/circleci/check_minified_errors.sh

test_build:
docker: *docker
environment: *environment
Expand Down Expand Up @@ -222,6 +232,9 @@ workflows:
- sizebot:
requires:
- build
- lint_build:
requires:
- build
- test_build:
requires:
- build
Expand Down
12 changes: 0 additions & 12 deletions scripts/circleci/build.sh

This file was deleted.

14 changes: 14 additions & 0 deletions scripts/circleci/check_minified_errors.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#!/bin/bash

# Ensure errors are minified in production

OUT=$(git --no-pager grep -n --untracked --no-exclude-standard 'FIXME (minify-errors-in-prod)' -- './build/*')

if [ "$OUT" != "" ]; then
echo "$OUT";
echo -e "\n";
echo "Detected an unminified error message in the production build. User-facing errors message must have a corresponding error code in scripts/error-codes/codes.json."
exit 1
fi

exit 0
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ exports[`error transform should correctly transform invariants that are not in t
"import _ReactError from 'shared/ReactError';
import invariant from 'shared/invariant';
(function () {
/*FIXME (minify-errors-in-prod): Unminified error message in production build!*/(function () {
if (!condition) {
throw _ReactError(\`This is not a real error message.\`);
}
Expand All @@ -15,7 +15,7 @@ exports[`error transform should handle escaped characters 1`] = `
"import _ReactError from 'shared/ReactError';
import invariant from 'shared/invariant';
(function () {
/*FIXME (minify-errors-in-prod): Unminified error message in production build!*/(function () {
if (!condition) {
throw _ReactError(\`What's up?\`);
}
Expand Down
30 changes: 26 additions & 4 deletions scripts/error-codes/transform-error-messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,19 +60,37 @@ module.exports = function(babel) {
])
);

if (noMinify) {
// Error minification is disabled for this build.
//
// Outputs:
// if (!condition) {
// throw ReactError(`A ${adj} message that contains ${noun}`);
// }
path.replaceWith(
t.ifStatement(
t.unaryExpression('!', condition),
t.blockStatement([devThrow])
)
);
return;
}

// Avoid caching because we write it as we go.
const existingErrorMap = JSON.parse(
fs.readFileSync(__dirname + '/codes.json', 'utf-8')
);
const errorMap = invertObject(existingErrorMap);

let prodErrorId = errorMap[errorMsgLiteral];
if (prodErrorId === undefined || noMinify) {
// There is no error code for this message. We use a lint rule to
// enforce that messages can be minified, so assume this is
// intentional and exit gracefully.

if (prodErrorId === undefined) {
// There is no error code for this message. Add an inline comment
// that flags this as an unminified error. This allows the build
// to proceed, while also allowing a post-build linter to detect it.
//
// Outputs:
// /* FIXME (minify-errors-in-prod): Unminified error message in production build! */
// if (!condition) {
// throw ReactError(`A ${adj} message that contains ${noun}`);
// }
Expand All @@ -82,6 +100,10 @@ module.exports = function(babel) {
t.blockStatement([devThrow])
)
);
path.addComment(
'leading',
'FIXME (minify-errors-in-prod): Unminified error message in production build!'
);
return;
}
prodErrorId = parseInt(prodErrorId, 10);
Expand Down

0 comments on commit fc36310

Please sign in to comment.