From 30b1a8009ce92d69b6debd1ff45dbb8a60f33b2f Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 29 May 2019 14:20:29 -0700 Subject: [PATCH] Don't extract errors in CI (#15758) 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. --- .circleci/config.yml | 17 ++++++- scripts/circleci/build.sh | 12 ----- scripts/circleci/check_minified_errors.sh | 14 ++++++ .../transform-error-messages.js.snap | 4 +- .../error-codes/transform-error-messages.js | 44 ++++++++++++++----- 5 files changed, 64 insertions(+), 27 deletions(-) delete mode 100755 scripts/circleci/build.sh create mode 100755 scripts/circleci/check_minified_errors.sh diff --git a/.circleci/config.yml b/.circleci/config.yml index 75f249e839..2d101d5063 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -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 @@ -145,7 +145,6 @@ jobs: - bundle-sizes.json sizebot: - build: docker: *docker environment: *environment steps: @@ -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 @@ -222,6 +232,9 @@ workflows: - sizebot: requires: - build + - lint_build: + requires: + - build - test_build: requires: - build diff --git a/scripts/circleci/build.sh b/scripts/circleci/build.sh deleted file mode 100755 index f6fae55e87..0000000000 --- a/scripts/circleci/build.sh +++ /dev/null @@ -1,12 +0,0 @@ -#!/bin/bash - -set -e - -yarn build --extract-errors -# Note: since we run the full build including extracting error codes, -# it is important that we *don't* reset the change to `scripts/error-codes/codes.json`. -# When production bundle tests run later, it needs to be available. -# See https://github.com/facebook/react/pull/11655. - -# Do a sanity check on bundles -yarn lint-build diff --git a/scripts/circleci/check_minified_errors.sh b/scripts/circleci/check_minified_errors.sh new file mode 100755 index 0000000000..df7673a6ea --- /dev/null +++ b/scripts/circleci/check_minified_errors.sh @@ -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 diff --git a/scripts/error-codes/__tests__/__snapshots__/transform-error-messages.js.snap b/scripts/error-codes/__tests__/__snapshots__/transform-error-messages.js.snap index 54eb6ec99d..3887fcc15b 100644 --- a/scripts/error-codes/__tests__/__snapshots__/transform-error-messages.js.snap +++ b/scripts/error-codes/__tests__/__snapshots__/transform-error-messages.js.snap @@ -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.\`); } @@ -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?\`); } diff --git a/scripts/error-codes/transform-error-messages.js b/scripts/error-codes/transform-error-messages.js index d83fdc4a59..f033157927 100644 --- a/scripts/error-codes/transform-error-messages.js +++ b/scripts/error-codes/transform-error-messages.js @@ -60,17 +60,8 @@ module.exports = function(babel) { ]) ); - // 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 (noMinify) { + // Error minification is disabled for this build. // // Outputs: // if (!condition) { @@ -84,6 +75,37 @@ module.exports = function(babel) { ); 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) { + // 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}`); + // } + path.replaceWith( + t.ifStatement( + t.unaryExpression('!', condition), + t.blockStatement([devThrow]) + ) + ); + path.addComment( + 'leading', + 'FIXME (minify-errors-in-prod): Unminified error message in production build!' + ); + return; + } prodErrorId = parseInt(prodErrorId, 10); // Import ReactErrorProd