From 8e280c8a7fde7f1ed11fcc1dcdbc6f3735876638 Mon Sep 17 00:00:00 2001 From: Flarnie Marchan Date: Fri, 11 Aug 2017 16:20:03 -0700 Subject: [PATCH 01/11] Throw error to warn of mistaken loading of prod + dev React bundles **what is the change?:** Credit to @gaearon for coming up with a clever way to check for this. :) I mainly just did the manual testing and fixed a couple of typos in his idea. I'd like to do a follow-up PR to add a link and a page explaining this issue more and suggesting how to fix it. **why make this change?:** We want to warn for an unfortunate gotcha for the following situation - 1. Wanting to shrink their JS bundle, an engineer runs it through 'uglifyjs' or some other minifier. They assume this will also do dead-code-elimination, but the 'prod' and 'dev' checks are not envified yet and dev-only code does not get removed. 2. Then they run it through browserify's 'envify' or some other tool to change all calls to 'process.env.NODE_ENV' to "production". This makes their code ready to ship, except the 'dev' only dead code is still in the bundle. Their bundle is twice as large as it needs to be, due to the dead code. This was a problem with the old build system before, but with our new build system output it's possible to detect and throw an informative error in this case. **test plan:** 1. run the build in react as usual; `yarn build` 2. manually run 'uglifyjs -mt' on 'build/packages/react/index.js' to simulate mistakenly minifying React before env variables are resolved, which skips dead code elimination. 3. run the fixtures build - `cd fixtures/packaging && node ./build-all.js && serve ../..` 4. Visit just the production browserify fixture - http://localhost:5000/fixtures/packaging/browserify/prod/ 5. You should see the error thrown indicating this problem has occurred. (Flarnie will insert a screenshot) 6. Do the above steps with NO uglifyjs step, and verify that no error is thrown. When there is no minification applied, we don't assume that this mix-up has occurred. (Flarnie will insert a screenshot) **issue:** fixes #9589 --- packages/react/index.js | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/packages/react/index.js b/packages/react/index.js index 999ead77897..0f5e0bd171a 100644 --- a/packages/react/index.js +++ b/packages/react/index.js @@ -1,6 +1,36 @@ 'use strict'; +function testMinification() { + if (testMinification.name === 'testMinification') { + // We are not minified. + // This might be a Node environment where DCE is not expected anyway. + return; + } + if (process.env.NODE_ENV === 'production') { + if (process.env.NODE_ENV === 'development') { + throw new Error('This is unreachable'); + } + try { + const source = testMinification.toString(); + if (source.indexOf('toString') === -1) { + // We know for a fact the above line exists. + // Therefore the browser gave us invalid source. + return; + } + if (source.indexOf('unreachable') !== -1) { + // Dead code elimination would have stripped that branch + // because it is impossible to reach in production. + setTimeout(function() { + // Ensure it gets reported to production logging + throw new Error('React is running in production mode, but dead code elimination has not been applied.'); + }); + } + } catch (e) {} + } +} + if (process.env.NODE_ENV === 'production') { + testMinification(); module.exports = require('./cjs/react.production.min.js'); } else { module.exports = require('./cjs/react.development.js'); From 441d661ca55bc6e2f5666eb19b2b844164970220 Mon Sep 17 00:00:00 2001 From: Flarnie Marchan Date: Sat, 12 Aug 2017 13:31:46 -0700 Subject: [PATCH 02/11] Remove extra 'prod' env. check and add link to docs in error message **what is the change?:** Based on helpful feedback from @gaearon - removed outer check that `process.env.NODE_ENV` is "production" since we are only calling the `testMinification` method inside of another check for "production" environment. - Added an fburl that points to [our current docs on using the production version of React](https://facebook.github.io/react/docs/optimizing-performance.html#use-the-production-build) **why make this change?:** To save an extra layer of conditionals and to make the error message more clear. **test plan:** Same test plan as earlier - 1. run the build in react as usual; yarn build 2. manually run 'uglifyjs -mt' on 'build/packages/react/index.js' to simulate mistakenly minifying React before env variables are resolved, which skips dead code elimination. 3. run the fixtures build - cd fixtures/packaging && node ./build-all.js && serve ../.. 4. Visit just the production browserify fixture - http://localhost:5000/fixtures/packaging/browserify/prod/ You should see the error thrown indicating this problem has occurred. (Flarnie will insert a screenshot in comments on the PR) 6. Do the above steps with NO uglifyjs step, and verify that no error is thrown. When there is no minification applied, we don't assume that this mix-up has occurred. (Flarnie will insert a screenshot in the comments on the PR.) **issue:** https://github.com/facebook/react/issues/9589 --- packages/react/index.js | 42 +++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/packages/react/index.js b/packages/react/index.js index 0f5e0bd171a..53b995601b6 100644 --- a/packages/react/index.js +++ b/packages/react/index.js @@ -6,27 +6,29 @@ function testMinification() { // This might be a Node environment where DCE is not expected anyway. return; } - if (process.env.NODE_ENV === 'production') { - if (process.env.NODE_ENV === 'development') { - throw new Error('This is unreachable'); - } - try { - const source = testMinification.toString(); - if (source.indexOf('toString') === -1) { - // We know for a fact the above line exists. - // Therefore the browser gave us invalid source. - return; - } - if (source.indexOf('unreachable') !== -1) { - // Dead code elimination would have stripped that branch - // because it is impossible to reach in production. - setTimeout(function() { - // Ensure it gets reported to production logging - throw new Error('React is running in production mode, but dead code elimination has not been applied.'); - }); - } - } catch (e) {} + if (process.env.NODE_ENV === 'development') { + // We expect this method only to be called in production. + throw new Error('This is unreachable'); } + try { + const source = testMinification.toString(); + if (source.indexOf('toString') === -1) { + // We know for a fact the above line exists. + // Therefore the browser gave us invalid source. + return; + } + if (source.indexOf('unreachable') !== -1) { + // Dead code elimination would have stripped that branch + // because it is impossible to reach in production. + setTimeout(function() { + // Ensure it gets reported to production logging + throw new Error('React is running in production mode, but dead code ' + + 'elimination has not been applied. Read how to correctly ' + + 'configure React for production: ' + + 'https://fburl.com/react-perf-use-the-production-build'); + }); + } + } catch (e) {} } if (process.env.NODE_ENV === 'production') { From 05be49a6cac908dade1776f9503234b70c21bae2 Mon Sep 17 00:00:00 2001 From: Flarnie Marchan Date: Sun, 13 Aug 2017 15:05:37 -0700 Subject: [PATCH 03/11] WIP fix and test 'testMinificationUsedDCE' method **what is the change?:** - Instead of looking for one match when having the method inspect it's own source, we look for two matches, because the search itself will be a match. - WIP moving this method into another file and testing it. Next steps: - Figure out why the babel.transform call is not actually minifying the code - Add tests for uglifyjs - Update build process so that this file can be accessed from `packages/react/index.js` **why make this change?:** - We had a bug in the 'testMinification' method, and I thought the name could be more clear. I fixed the code and also changed the name. - In order to avoid other bugs and keep this code working in the future we'd like to add a unit test for this method. Using the npm modules for 'uglifyjs' and 'babel'/'babili' we should be able to actually test how this method will work when minified with different configurations. **test plan:** `yarn test` **issue:** https://github.com/facebook/react/issues/9589 --- packages/react/index.js | 33 +------ .../__tests__/testMinificationUsedDCE-test.js | 86 +++++++++++++++++++ src/shared/utils/testMinificationUsedDCE.js | 56 ++++++++++++ 3 files changed, 144 insertions(+), 31 deletions(-) create mode 100644 src/shared/utils/__tests__/testMinificationUsedDCE-test.js create mode 100644 src/shared/utils/testMinificationUsedDCE.js diff --git a/packages/react/index.js b/packages/react/index.js index 53b995601b6..42e5f9ba5c4 100644 --- a/packages/react/index.js +++ b/packages/react/index.js @@ -1,38 +1,9 @@ 'use strict'; -function testMinification() { - if (testMinification.name === 'testMinification') { - // We are not minified. - // This might be a Node environment where DCE is not expected anyway. - return; - } - if (process.env.NODE_ENV === 'development') { - // We expect this method only to be called in production. - throw new Error('This is unreachable'); - } - try { - const source = testMinification.toString(); - if (source.indexOf('toString') === -1) { - // We know for a fact the above line exists. - // Therefore the browser gave us invalid source. - return; - } - if (source.indexOf('unreachable') !== -1) { - // Dead code elimination would have stripped that branch - // because it is impossible to reach in production. - setTimeout(function() { - // Ensure it gets reported to production logging - throw new Error('React is running in production mode, but dead code ' - + 'elimination has not been applied. Read how to correctly ' - + 'configure React for production: ' - + 'https://fburl.com/react-perf-use-the-production-build'); - }); - } - } catch (e) {} -} if (process.env.NODE_ENV === 'production') { - testMinification(); + // TODO: actually update the build process so this works. + (require('./cjs/testMinificationUsedDCE.js'))(); module.exports = require('./cjs/react.production.min.js'); } else { module.exports = require('./cjs/react.development.js'); diff --git a/src/shared/utils/__tests__/testMinificationUsedDCE-test.js b/src/shared/utils/__tests__/testMinificationUsedDCE-test.js new file mode 100644 index 00000000000..ff7bf6bf2c9 --- /dev/null +++ b/src/shared/utils/__tests__/testMinificationUsedDCE-test.js @@ -0,0 +1,86 @@ +/** + * Copyright 2013-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @emails react-core + */ +'use strict'; + +var babel = require('babel-core'); +//var minify = require('babel-preset-minify'); +var testMinificationUsedDCE; + + +describe('in the production environment', () => { + let oldProcess; + beforeEach(() => { + __DEV__ = false; + + // Mutating process.env.NODE_ENV would cause our babel plugins to do the + // wrong thing. If you change this, make sure to test with jest --no-cache. + oldProcess = process; + global.process = { + ...process, + env: {...process.env, NODE_ENV: 'production'}, + }; + + jest.resetModules(); + testMinificationUsedDCE = require('testMinificationUsedDCE'); + }); + + afterEach(() => { + __DEV__ = true; + global.process = oldProcess; + + }); + + describe('when not minified', () => { + it('should not throw', () => { + expect(() => { + testMinificationUsedDCE(); + jest.runAllTimers(); + }).not.toThrow(); + }); + }); + + describe('when minified with babel/minify with *no* DCE', () => { + xit('should throw', () => { + const babelOpts = { + presets: ['babel-preset-minify'], + }; + // TODO: Why is this not actually minifying the code???? + const minifiedWithNoDCE = () => { + eval( + babel.transform(testMinificationUsedDCE.toString(), babelOpts).code + ); + }; + expect(() => { + minifiedWithNoDCE(); + jest.runAllTimers(); + }).toThrow(); + }); + }); + + describe('when minified with babel/minify with DCE', () => { + xit('should not throw', () => { + const babelOpts = { + plugins: ['minify-dead-code-elimination'], + presets: ['babel-preset-minify'], + }; + // TODO: Why is this not actually minifying the code???? + const minifiedWithDCE = () => { + eval( + babel.transform(testMinificationUsedDCE.toString(), babelOpts).code + ); + }; + expect(() => { + testMinificationUsedDCE(); + jest.runAllTimers(); + }).not.toThrow(); + }); + }); +}); diff --git a/src/shared/utils/testMinificationUsedDCE.js b/src/shared/utils/testMinificationUsedDCE.js new file mode 100644 index 00000000000..83acd232148 --- /dev/null +++ b/src/shared/utils/testMinificationUsedDCE.js @@ -0,0 +1,56 @@ +/** + * Copyright 2014-2015, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @providesModule testMinificationUsedDCE + */ + +'use strict'; + +function testMinificationUsedDCE() { + if (process.env.NODE_ENV === 'production') { + const source = testMinificationUsedDCE.toString(); + const longVariableName = source; + if (longVariableName && + source.match(/longVariableName/g).length === 3) { + // We are not minified. + // This might be a Node environment where DCE is not expected anyway. + return; + } + if (process.env.NODE_ENV === 'development') { + // We expect this method only to be called in production. + throw new Error('This is unreachable'); + } + try { + if (source.match(/toString/g).length !== 2) { + // We always look for two matches: + // The actual occurence and then the call to 'match' + // + // We know for a fact the above line exists so there should be 2 + // matches. + // Therefore the browser gave us invalid source. + return; + } + if (source.match(/unreachable/g).length !== 2) { + // We always look for two matches: + // The actual occurence and then the call to 'match' + + // Dead code elimination would have stripped that branch + // because it is impossible to reach in production. + setTimeout(function() { + // Ensure it gets reported to production logging + throw new Error('React is running in production mode, but dead code ' + + 'elimination has not been applied. Read how to correctly ' + + 'configure React for production: ' + + 'https://fburl.com/react-perf-use-the-production-build'); + }); + } + } catch (e) {} + } +} + +module.exports = testMinificationUsedDCE; From 8fab8851bb2569e86cab98ce386783bb2a19f475 Mon Sep 17 00:00:00 2001 From: Flarnie Marchan Date: Sun, 13 Aug 2017 15:15:14 -0700 Subject: [PATCH 04/11] Add babeli and uglifyjs as dev-only dependencies **what is the change?:** See commit title **why make this change?:** In order to test that we are correctly detecting different minification situations, we are using these modules in a test. **test plan:** NA **issue:** https://github.com/facebook/react/issues/9589 --- package.json | 4 ++- yarn.lock | 77 +++++++++++++++++++++++++++++++++++++++------------- 2 files changed, 61 insertions(+), 20 deletions(-) diff --git a/package.json b/package.json index 90511293b6a..4c18b61bde4 100644 --- a/package.json +++ b/package.json @@ -11,6 +11,7 @@ "babel-jest": "20.1.0-delta.1", "babel-plugin-check-es2015-constants": "^6.5.0", "babel-plugin-external-helpers": "^6.22.0", + "babel-plugin-minify-dead-code-elimination": "^0.1.7", "babel-plugin-syntax-trailing-function-commas": "^6.5.0", "babel-plugin-transform-async-to-generator": "^6.22.0", "babel-plugin-transform-class-properties": "^6.11.5", @@ -32,6 +33,7 @@ "babel-plugin-transform-es3-property-literals": "^6.5.0", "babel-plugin-transform-object-rest-spread": "^6.6.5", "babel-plugin-transform-react-jsx-source": "^6.8.0", + "babel-preset-minify": "0.0.0", "babel-preset-react": "^6.5.0", "babel-traverse": "^6.9.0", "babylon": "6.15.0", @@ -86,7 +88,7 @@ "through2": "^2.0.0", "tmp": "~0.0.28", "typescript": "~1.8.10", - "uglify-js": "^2.5.0", + "uglify-js": "^2.8.29", "yargs": "^6.3.0" }, "devEngines": { diff --git a/yarn.lock b/yarn.lock index 43a4ec685dd..6726c50acd0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -417,6 +417,10 @@ babel-helper-hoist-variables@^6.24.1: babel-runtime "^6.22.0" babel-types "^6.24.1" +babel-helper-mark-eval-scopes@^0.1.1: + version "0.1.1" + resolved "https://registry.yarnpkg.com/babel-helper-mark-eval-scopes/-/babel-helper-mark-eval-scopes-0.1.1.tgz#4554345edf9f2549427bd2098e530253f8af2992" + babel-helper-optimise-call-expression@^6.24.1: version "6.24.1" resolved "https://registry.yarnpkg.com/babel-helper-optimise-call-expression/-/babel-helper-optimise-call-expression-6.24.1.tgz#f7a13427ba9f73f8f4fa993c54a97882d1244257" @@ -434,6 +438,10 @@ babel-helper-remap-async-to-generator@^6.24.1: babel-traverse "^6.24.1" babel-types "^6.24.1" +babel-helper-remove-or-void@^0.1.1: + version "0.1.1" + resolved "https://registry.yarnpkg.com/babel-helper-remove-or-void/-/babel-helper-remove-or-void-0.1.1.tgz#9d7e1856dc6fafcb41b283a416730dc1844f66d7" + babel-helper-replace-supers@^6.24.1: version "6.24.1" resolved "https://registry.yarnpkg.com/babel-helper-replace-supers/-/babel-helper-replace-supers-6.24.1.tgz#bf6dbfe43938d17369a213ca8a8bf74b6a90ab1a" @@ -514,6 +522,14 @@ babel-plugin-member-expression-literals@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/babel-plugin-member-expression-literals/-/babel-plugin-member-expression-literals-1.0.1.tgz#cc5edb0faa8dc927170e74d6d1c02440021624d3" +babel-plugin-minify-dead-code-elimination@^0.1.7: + version "0.1.7" + resolved "https://registry.yarnpkg.com/babel-plugin-minify-dead-code-elimination/-/babel-plugin-minify-dead-code-elimination-0.1.7.tgz#774f536f347b98393a27baa717872968813c342c" + dependencies: + babel-helper-mark-eval-scopes "^0.1.1" + babel-helper-remove-or-void "^0.1.1" + lodash.some "^4.6.0" + babel-plugin-property-literals@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/babel-plugin-property-literals/-/babel-plugin-property-literals-1.0.1.tgz#0252301900192980b1c118efea48ce93aab83336" @@ -711,6 +727,14 @@ babel-plugin-transform-flow-strip-types@^6.22.0: babel-plugin-syntax-flow "^6.18.0" babel-runtime "^6.22.0" +babel-plugin-transform-member-expression-literals@^6.8.0: + version "6.8.4" + resolved "https://registry.yarnpkg.com/babel-plugin-transform-member-expression-literals/-/babel-plugin-transform-member-expression-literals-6.8.4.tgz#05679bc40596b91293401959aa1620ab1b2be437" + +babel-plugin-transform-merge-sibling-variables@^6.8.0: + version "6.8.5" + resolved "https://registry.yarnpkg.com/babel-plugin-transform-merge-sibling-variables/-/babel-plugin-transform-merge-sibling-variables-6.8.5.tgz#03abdf107c61241913eb268ddede6d5bc541862c" + babel-plugin-transform-object-rest-spread@^6.6.5: version "6.23.0" resolved "https://registry.yarnpkg.com/babel-plugin-transform-object-rest-spread/-/babel-plugin-transform-object-rest-spread-6.23.0.tgz#875d6bc9be761c58a2ae3feee5dc4895d8c7f921" @@ -718,6 +742,12 @@ babel-plugin-transform-object-rest-spread@^6.6.5: babel-plugin-syntax-object-rest-spread "^6.8.0" babel-runtime "^6.22.0" +babel-plugin-transform-property-literals@^6.8.0: + version "6.8.4" + resolved "https://registry.yarnpkg.com/babel-plugin-transform-property-literals/-/babel-plugin-transform-property-literals-6.8.4.tgz#6ad311110b80a192a56efb5ddf4fe3ca6f7a61da" + dependencies: + esutils "^2.0.2" + babel-plugin-transform-react-display-name@^6.23.0: version "6.23.0" resolved "https://registry.yarnpkg.com/babel-plugin-transform-react-display-name/-/babel-plugin-transform-react-display-name-6.23.0.tgz#4398910c358441dc4cef18787264d0412ed36b37" @@ -783,6 +813,14 @@ babel-preset-jest@20.1.0-delta.1: dependencies: babel-plugin-jest-hoist "20.1.0-delta.1" +babel-preset-minify@0.0.0: + version "0.0.0" + resolved "https://registry.yarnpkg.com/babel-preset-minify/-/babel-preset-minify-0.0.0.tgz#ccede0e043f80c488d65d8fe7b3ec6c2c68d284a" + dependencies: + babel-plugin-transform-member-expression-literals "^6.8.0" + babel-plugin-transform-merge-sibling-variables "^6.8.0" + babel-plugin-transform-property-literals "^6.8.0" + babel-preset-react@^6.5.0: version "6.24.1" resolved "https://registry.yarnpkg.com/babel-preset-react/-/babel-preset-react-6.24.1.tgz#ba69dfaea45fc3ec639b6a4ecea6e17702c91380" @@ -2078,23 +2116,7 @@ google-closure-compiler-js@>20170000: vinyl "^2.0.1" webpack-core "^0.6.8" -got@^6.7.1: - version "6.7.1" - resolved "https://registry.yarnpkg.com/got/-/got-6.7.1.tgz#240cd05785a9a18e561dc1b44b41c763ef1e8db0" - dependencies: - create-error-class "^3.0.0" - duplexer3 "^0.1.4" - get-stream "^3.0.0" - is-redirect "^1.0.0" - is-retry-allowed "^1.0.0" - is-stream "^1.0.0" - lowercase-keys "^1.0.0" - safe-buffer "^5.0.1" - timed-out "^4.0.0" - unzip-response "^2.0.1" - url-parse-lax "^1.0.0" - -graceful-fs@^4.1.11, graceful-fs@^4.1.2, graceful-fs@^4.1.4, graceful-fs@^4.1.6, graceful-fs@^4.1.9: +graceful-fs@^4.1.11, graceful-fs@^4.1.2, graceful-fs@^4.1.4: version "4.1.11" resolved "https://registry.yarnpkg.com/graceful-fs/-/graceful-fs-4.1.11.tgz#0e8bdfe4d1ddb8854d64e04ea7c00e2a026e5658" @@ -3084,6 +3106,10 @@ lodash.restparam@^3.0.0: version "3.6.1" resolved "https://registry.yarnpkg.com/lodash.restparam/-/lodash.restparam-3.6.1.tgz#936a4e309ef330a7645ed4145986c85ae5b20805" +lodash.some@^4.6.0: + version "4.6.0" + resolved "https://registry.yarnpkg.com/lodash.some/-/lodash.some-4.6.0.tgz#1bb9f314ef6b8baded13b549169b2a945eb68e4d" + lodash.template@^3.0.0: version "3.6.2" resolved "https://registry.yarnpkg.com/lodash.template/-/lodash.template-3.6.2.tgz#f8cdecc6169a255be9098ae8b0c53d378931d14f" @@ -3818,7 +3844,7 @@ replace-ext@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/replace-ext/-/replace-ext-1.0.0.tgz#de63128373fcbf7c3ccfa4de5a480c45a67958eb" -request@2, request@2.79.0: +request@2.79.0: version "2.79.0" resolved "https://registry.yarnpkg.com/request/-/request-2.79.0.tgz#4dfe5bf6be8b8cdc37fcf93e04b65577722710de" dependencies: @@ -4431,11 +4457,15 @@ typedarray@~0.0.5: version "0.0.6" resolved "https://registry.yarnpkg.com/typedarray/-/typedarray-0.0.6.tgz#867ac74e3864187b1d3d47d996a78ec5c8830777" +typescript@~1.8.10: + version "1.8.10" + resolved "https://registry.yarnpkg.com/typescript/-/typescript-1.8.10.tgz#b475d6e0dff0bf50f296e5ca6ef9fbb5c7320f1e" + ua-parser-js@^0.7.9: version "0.7.12" resolved "https://registry.yarnpkg.com/ua-parser-js/-/ua-parser-js-0.7.12.tgz#04c81a99bdd5dc52263ea29d24c6bf8d4818a4bb" -uglify-js@^2.5.0, uglify-js@^2.6, uglify-js@^2.6.1: +uglify-js@^2.6, uglify-js@^2.6.1: version "2.8.22" resolved "https://registry.yarnpkg.com/uglify-js/-/uglify-js-2.8.22.tgz#d54934778a8da14903fa29a326fb24c0ab51a1a0" dependencies: @@ -4444,6 +4474,15 @@ uglify-js@^2.5.0, uglify-js@^2.6, uglify-js@^2.6.1: optionalDependencies: uglify-to-browserify "~1.0.0" +uglify-js@^2.8.29: + version "2.8.29" + resolved "https://registry.yarnpkg.com/uglify-js/-/uglify-js-2.8.29.tgz#29c5733148057bb4e1f75df35b7a9cb72e6a59dd" + dependencies: + source-map "~0.5.1" + yargs "~3.10.0" + optionalDependencies: + uglify-to-browserify "~1.0.0" + uglify-to-browserify@~1.0.0: version "1.0.2" resolved "https://registry.yarnpkg.com/uglify-to-browserify/-/uglify-to-browserify-1.0.2.tgz#6e0924d6bda6b5afe349e39a6d632850a0f882b7" From 9a73704e19abc7b96b1ae28c3df0bcc714d1ecbd Mon Sep 17 00:00:00 2001 From: Flarnie Marchan Date: Sun, 13 Aug 2017 17:10:39 -0700 Subject: [PATCH 05/11] Fix typo and add 'uglifyjs' tests **what is the change?:** There was a mix-up in the conditional in 'testMinificationUsedDCE' and this fixes it. The great new tests we added caught the typo. :) Next steps: - get the tests for 'babili' working - update build scripts so that this the 'testMinificationUsedDCE' module is available from 'packages/react/index.js' **why make this change?:** We want to modularize 'testMinificationUsedDCE' and test it. This verifies that the method warns when `uglifyjs` is used to minify but dead code elimination is not done, in a production environment. Generally this is a 'gotcha' which we want to warn folks aboug. **test plan:** `yarn test src/shared/utils/__tests__/testMinificationUsedDCE-test.js` **issue:** https://github.com/facebook/react/issues/9589 --- .../__tests__/testMinificationUsedDCE-test.js | 61 ++++++++++++++++++- src/shared/utils/testMinificationUsedDCE.js | 4 +- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/src/shared/utils/__tests__/testMinificationUsedDCE-test.js b/src/shared/utils/__tests__/testMinificationUsedDCE-test.js index ff7bf6bf2c9..46a92ea5f81 100644 --- a/src/shared/utils/__tests__/testMinificationUsedDCE-test.js +++ b/src/shared/utils/__tests__/testMinificationUsedDCE-test.js @@ -11,7 +11,7 @@ 'use strict'; var babel = require('babel-core'); -//var minify = require('babel-preset-minify'); +var UglifyJs = require('uglify-js'); var testMinificationUsedDCE; @@ -47,6 +47,65 @@ describe('in the production environment', () => { }); }); + describe('with uglifyjs', () => { + describe('when envified first, then minification with *no* DCE', () => { + it('should throw', () => { + // envify: change `process.env.NODE_ENV` to `"production"` + var rawCode = testMinificationUsedDCE + .toString() + .replace(/process\.env\.NODE_ENV/, '"production"'); + var code = { 'file.js': rawCode }; + var options = { fromString: true, parse: { dead_code: false } }; + var result = UglifyJs.minify(code, options); + const minifiedWithNoDCE = () => eval('(' + result.code + ')()'); + expect(() => { + minifiedWithNoDCE(); + jest.runAllTimers(); + }).toThrow( + 'React is running in production mode, but dead code elimination ' + + 'has not been applied.', + ); + }); + }); + + describe('when envified first, then minified and *yes* successful DCE', () => { + it('should not throw', () => { + // envify: change `process.env.NODE_ENV` to `"production"` + var rawCode = testMinificationUsedDCE + .toString() + .replace(/process\.env\.NODE_ENV/g, '"production"'); + var code = { 'file.js': rawCode }; + var options = { fromString: true, parse: { dead_code: true } }; + var result = UglifyJs.minify(code, options); + const minifiedWithNoDCE = () => eval('(' + result.code + ')()'); + expect(() => { + minifiedWithNoDCE(); + jest.runAllTimers(); + }).not.toThrow(); + }); + }); + + describe('when minified first with *unsuccessful* DCE, then envified', () => { + it('should throw', () => { + var code = { 'file.js': testMinificationUsedDCE.toString() }; + var options = { fromString: true, parse: { dead_code: true } }; + var result = UglifyJs.minify(code, options); + // late envify: change `process.env.NODE_ENV` to `"production"` + var resultCode = result.code + .replace(/process\.env\.NODE_ENV/g, '"production"'); + const minifiedWithNoDCE = () => eval('(' + resultCode + ')()'); + expect(() => { + minifiedWithNoDCE(); + jest.runAllTimers(); + }).toThrow( + 'React is running in production mode, but dead code elimination ' + + 'has not been applied.', + ); + }); + }); + }); + + describe('when minified with babel/minify with *no* DCE', () => { xit('should throw', () => { const babelOpts = { diff --git a/src/shared/utils/testMinificationUsedDCE.js b/src/shared/utils/testMinificationUsedDCE.js index 83acd232148..e705479fa8f 100644 --- a/src/shared/utils/testMinificationUsedDCE.js +++ b/src/shared/utils/testMinificationUsedDCE.js @@ -13,6 +13,8 @@ function testMinificationUsedDCE() { if (process.env.NODE_ENV === 'production') { + // use scoped variable for our initial test, in case + // 'top-level' mangling is not enabled. const source = testMinificationUsedDCE.toString(); const longVariableName = source; if (longVariableName && @@ -35,7 +37,7 @@ function testMinificationUsedDCE() { // Therefore the browser gave us invalid source. return; } - if (source.match(/unreachable/g).length !== 2) { + if (source.match(/unreachable/g).length === 2) { // We always look for two matches: // The actual occurence and then the call to 'match' From 5204310bf8cb43431c162ee0bca48ba4490529ac Mon Sep 17 00:00:00 2001 From: Flarnie Marchan Date: Sun, 13 Aug 2017 17:21:04 -0700 Subject: [PATCH 06/11] Run prettier --- .../__tests__/testMinificationUsedDCE-test.js | 33 +++++++++---------- src/shared/utils/testMinificationUsedDCE.js | 13 ++++---- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/shared/utils/__tests__/testMinificationUsedDCE-test.js b/src/shared/utils/__tests__/testMinificationUsedDCE-test.js index 46a92ea5f81..b50a8833ebb 100644 --- a/src/shared/utils/__tests__/testMinificationUsedDCE-test.js +++ b/src/shared/utils/__tests__/testMinificationUsedDCE-test.js @@ -14,7 +14,6 @@ var babel = require('babel-core'); var UglifyJs = require('uglify-js'); var testMinificationUsedDCE; - describe('in the production environment', () => { let oldProcess; beforeEach(() => { @@ -35,7 +34,6 @@ describe('in the production environment', () => { afterEach(() => { __DEV__ = true; global.process = oldProcess; - }); describe('when not minified', () => { @@ -54,16 +52,16 @@ describe('in the production environment', () => { var rawCode = testMinificationUsedDCE .toString() .replace(/process\.env\.NODE_ENV/, '"production"'); - var code = { 'file.js': rawCode }; - var options = { fromString: true, parse: { dead_code: false } }; + var code = {'file.js': rawCode}; + var options = {fromString: true, parse: {dead_code: false}}; var result = UglifyJs.minify(code, options); const minifiedWithNoDCE = () => eval('(' + result.code + ')()'); expect(() => { minifiedWithNoDCE(); jest.runAllTimers(); }).toThrow( - 'React is running in production mode, but dead code elimination ' - + 'has not been applied.', + 'React is running in production mode, but dead code elimination ' + + 'has not been applied.', ); }); }); @@ -74,8 +72,8 @@ describe('in the production environment', () => { var rawCode = testMinificationUsedDCE .toString() .replace(/process\.env\.NODE_ENV/g, '"production"'); - var code = { 'file.js': rawCode }; - var options = { fromString: true, parse: { dead_code: true } }; + var code = {'file.js': rawCode}; + var options = {fromString: true, parse: {dead_code: true}}; var result = UglifyJs.minify(code, options); const minifiedWithNoDCE = () => eval('(' + result.code + ')()'); expect(() => { @@ -87,25 +85,26 @@ describe('in the production environment', () => { describe('when minified first with *unsuccessful* DCE, then envified', () => { it('should throw', () => { - var code = { 'file.js': testMinificationUsedDCE.toString() }; - var options = { fromString: true, parse: { dead_code: true } }; + var code = {'file.js': testMinificationUsedDCE.toString()}; + var options = {fromString: true, parse: {dead_code: true}}; var result = UglifyJs.minify(code, options); // late envify: change `process.env.NODE_ENV` to `"production"` - var resultCode = result.code - .replace(/process\.env\.NODE_ENV/g, '"production"'); + var resultCode = result.code.replace( + /process\.env\.NODE_ENV/g, + '"production"', + ); const minifiedWithNoDCE = () => eval('(' + resultCode + ')()'); expect(() => { minifiedWithNoDCE(); jest.runAllTimers(); }).toThrow( - 'React is running in production mode, but dead code elimination ' - + 'has not been applied.', + 'React is running in production mode, but dead code elimination ' + + 'has not been applied.', ); }); }); }); - describe('when minified with babel/minify with *no* DCE', () => { xit('should throw', () => { const babelOpts = { @@ -114,7 +113,7 @@ describe('in the production environment', () => { // TODO: Why is this not actually minifying the code???? const minifiedWithNoDCE = () => { eval( - babel.transform(testMinificationUsedDCE.toString(), babelOpts).code + babel.transform(testMinificationUsedDCE.toString(), babelOpts).code, ); }; expect(() => { @@ -133,7 +132,7 @@ describe('in the production environment', () => { // TODO: Why is this not actually minifying the code???? const minifiedWithDCE = () => { eval( - babel.transform(testMinificationUsedDCE.toString(), babelOpts).code + babel.transform(testMinificationUsedDCE.toString(), babelOpts).code, ); }; expect(() => { diff --git a/src/shared/utils/testMinificationUsedDCE.js b/src/shared/utils/testMinificationUsedDCE.js index e705479fa8f..344a612338a 100644 --- a/src/shared/utils/testMinificationUsedDCE.js +++ b/src/shared/utils/testMinificationUsedDCE.js @@ -17,8 +17,7 @@ function testMinificationUsedDCE() { // 'top-level' mangling is not enabled. const source = testMinificationUsedDCE.toString(); const longVariableName = source; - if (longVariableName && - source.match(/longVariableName/g).length === 3) { + if (longVariableName && source.match(/longVariableName/g).length === 3) { // We are not minified. // This might be a Node environment where DCE is not expected anyway. return; @@ -45,10 +44,12 @@ function testMinificationUsedDCE() { // because it is impossible to reach in production. setTimeout(function() { // Ensure it gets reported to production logging - throw new Error('React is running in production mode, but dead code ' - + 'elimination has not been applied. Read how to correctly ' - + 'configure React for production: ' - + 'https://fburl.com/react-perf-use-the-production-build'); + throw new Error( + 'React is running in production mode, but dead code ' + + 'elimination has not been applied. Read how to correctly ' + + 'configure React for production: ' + + 'https://fburl.com/react-perf-use-the-production-build', + ); }); } } catch (e) {} From 26ce0acbb72e96c16cb2ac1b983f61d7882998d7 Mon Sep 17 00:00:00 2001 From: Flarnie Marchan Date: Sun, 13 Aug 2017 17:28:32 -0700 Subject: [PATCH 07/11] var -> const/let --- .../__tests__/testMinificationUsedDCE-test.js | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/shared/utils/__tests__/testMinificationUsedDCE-test.js b/src/shared/utils/__tests__/testMinificationUsedDCE-test.js index b50a8833ebb..29fcce2bfdb 100644 --- a/src/shared/utils/__tests__/testMinificationUsedDCE-test.js +++ b/src/shared/utils/__tests__/testMinificationUsedDCE-test.js @@ -10,9 +10,9 @@ */ 'use strict'; -var babel = require('babel-core'); -var UglifyJs = require('uglify-js'); -var testMinificationUsedDCE; +const babel = require('babel-core'); +const UglifyJs = require('uglify-js'); +let testMinificationUsedDCE; describe('in the production environment', () => { let oldProcess; @@ -49,12 +49,12 @@ describe('in the production environment', () => { describe('when envified first, then minification with *no* DCE', () => { it('should throw', () => { // envify: change `process.env.NODE_ENV` to `"production"` - var rawCode = testMinificationUsedDCE + const rawCode = testMinificationUsedDCE .toString() .replace(/process\.env\.NODE_ENV/, '"production"'); - var code = {'file.js': rawCode}; - var options = {fromString: true, parse: {dead_code: false}}; - var result = UglifyJs.minify(code, options); + const code = {'file.js': rawCode}; + const options = {fromString: true, parse: {dead_code: false}}; + const result = UglifyJs.minify(code, options); const minifiedWithNoDCE = () => eval('(' + result.code + ')()'); expect(() => { minifiedWithNoDCE(); @@ -69,12 +69,12 @@ describe('in the production environment', () => { describe('when envified first, then minified and *yes* successful DCE', () => { it('should not throw', () => { // envify: change `process.env.NODE_ENV` to `"production"` - var rawCode = testMinificationUsedDCE + const rawCode = testMinificationUsedDCE .toString() .replace(/process\.env\.NODE_ENV/g, '"production"'); - var code = {'file.js': rawCode}; - var options = {fromString: true, parse: {dead_code: true}}; - var result = UglifyJs.minify(code, options); + const code = {'file.js': rawCode}; + const options = {fromString: true, parse: {dead_code: true}}; + const result = UglifyJs.minify(code, options); const minifiedWithNoDCE = () => eval('(' + result.code + ')()'); expect(() => { minifiedWithNoDCE(); @@ -85,11 +85,11 @@ describe('in the production environment', () => { describe('when minified first with *unsuccessful* DCE, then envified', () => { it('should throw', () => { - var code = {'file.js': testMinificationUsedDCE.toString()}; - var options = {fromString: true, parse: {dead_code: true}}; - var result = UglifyJs.minify(code, options); + const code = {'file.js': testMinificationUsedDCE.toString()}; + const options = {fromString: true, parse: {dead_code: true}}; + const result = UglifyJs.minify(code, options); // late envify: change `process.env.NODE_ENV` to `"production"` - var resultCode = result.code.replace( + const resultCode = result.code.replace( /process\.env\.NODE_ENV/g, '"production"', ); From 498f8f5b0574e248ebd2bad1619246b5e49a0c50 Mon Sep 17 00:00:00 2001 From: Flarnie Marchan Date: Mon, 14 Aug 2017 08:04:07 -0700 Subject: [PATCH 08/11] Require specific version of `uglify-js` **what is the change?:** Removed the '^' from the npm package requirement **why make this change?:** I am seeing failures in CI that show `uglify-js` is returning different output there from my local environment. To further debug this I'd like to run CI with the exact same version of `uglify-js` that I am using locally. **test plan:** push and see what CI does **issue:** https://github.com/facebook/react/issues/9589 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 4c18b61bde4..86613d9225a 100644 --- a/package.json +++ b/package.json @@ -88,7 +88,7 @@ "through2": "^2.0.0", "tmp": "~0.0.28", "typescript": "~1.8.10", - "uglify-js": "^2.8.29", + "uglify-js": "2.8.29", "yargs": "^6.3.0" }, "devEngines": { From ab6c1f0fa37e0837494fdc779e96c76110d395d3 Mon Sep 17 00:00:00 2001 From: Flarnie Marchan Date: Mon, 14 Aug 2017 09:39:26 -0700 Subject: [PATCH 09/11] Add build step copying testMinificationUsedDCE into build/packages/react/cj **what is the change?:** This is a first step - we still need (I think) to process this file to get it's contents wrapped in an 'iffe'. Added a step to the build script which copies the source file for the 'testMinificationUsedDCE' module into the 'cjs' directory of our react package build. **why make this change?:** We want this module to be available to the 'index.js' module in this build. **test plan:** Will do manual testing once the build stuff is fully set up. **issue:** --- scripts/rollup/packaging.js | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/scripts/rollup/packaging.js b/scripts/rollup/packaging.js index e5cf6f7eae6..8e9d60d5d7a 100644 --- a/scripts/rollup/packaging.js +++ b/scripts/rollup/packaging.js @@ -30,6 +30,11 @@ const reactNativeSrcDependencies = [ 'src/renderers/native/ReactNativeTypes.js', ]; +// these files need to be copied to the react build +const reactPackageDependencies = [ + 'src/shared/utils/testMinificationUsedDCE.js', +]; + function getPackageName(name) { if (name.indexOf('/') !== -1) { return name.split('/')[0]; @@ -99,6 +104,7 @@ function copyBundleIntoNodePackage(packageName, filename, bundleType) { from = resolve(`./build/dist/${filename}`); to = `${packageDirectory}/umd/${filename}`; } + const promises = []; // for NODE bundles we have to move the files into a cjs directory // within the package directory. we also need to set the from // to be the root build from directory @@ -109,13 +115,31 @@ function copyBundleIntoNodePackage(packageName, filename, bundleType) { fs.mkdirSync(distDirectory); } to = `${packageDirectory}/cjs/${filename}`; - } - return asyncCopyTo(from, to).then(() => { - // delete the old file if this is a not a UMD bundle - if (bundleType !== UMD_DEV && bundleType !== UMD_PROD) { - fs.unlinkSync(from); + if (packageName === 'react') { + // we are building the React package + let promises = []; + // we also need to copy over some specific files from src + // defined in reactPackageDependencies + for (const srcDependency of reactPackageDependencies) { + // TODO: the srcDependency needs to be processed + // before this step. + var specificFrom = resolve(srcDependency); + var specificTo = resolve(`${distDirectory}/${basename(srcDependency)}`); + promises.push( + asyncCopyTo(specificFrom, specificTo) + ); + } } - }); + } + promises.push( + asyncCopyTo(from, to).then(() => { + // delete the old file if this is a not a UMD bundle + if (bundleType !== UMD_DEV && bundleType !== UMD_PROD) { + fs.unlinkSync(from); + } + }) + ); + return Promise.all(promises); } else { return Promise.resolve(); } From 8a1c36b7a09e5b4d93fa8b4fca31d66947109478 Mon Sep 17 00:00:00 2001 From: Flarnie Marchan Date: Mon, 14 Aug 2017 11:18:55 -0700 Subject: [PATCH 10/11] Inline 'testMinificationUsedDCE' and remove unit test for now What: - Inlines the 'testMinificationUsedDCE' method into 'packages/react/index.js' - Removes unit test for 'testMinififcationUsedDCE' - Puts dependencies back the way that they were; should remove extra dependencies that were added for the unit test. Why: - It would add complexity to the build process to add another file to the 'build/packages/react/cjs' folder, and that is the only way to pull this out and test it. So instead we are inlining this. --- package.json | 4 +- packages/react/index.js | 45 +++++- scripts/rollup/packaging.js | 37 +---- .../__tests__/testMinificationUsedDCE-test.js | 144 ------------------ src/shared/utils/testMinificationUsedDCE.js | 59 ------- 5 files changed, 51 insertions(+), 238 deletions(-) delete mode 100644 src/shared/utils/__tests__/testMinificationUsedDCE-test.js delete mode 100644 src/shared/utils/testMinificationUsedDCE.js diff --git a/package.json b/package.json index 86613d9225a..69e8fec2e56 100644 --- a/package.json +++ b/package.json @@ -11,7 +11,6 @@ "babel-jest": "20.1.0-delta.1", "babel-plugin-check-es2015-constants": "^6.5.0", "babel-plugin-external-helpers": "^6.22.0", - "babel-plugin-minify-dead-code-elimination": "^0.1.7", "babel-plugin-syntax-trailing-function-commas": "^6.5.0", "babel-plugin-transform-async-to-generator": "^6.22.0", "babel-plugin-transform-class-properties": "^6.11.5", @@ -33,7 +32,6 @@ "babel-plugin-transform-es3-property-literals": "^6.5.0", "babel-plugin-transform-object-rest-spread": "^6.6.5", "babel-plugin-transform-react-jsx-source": "^6.8.0", - "babel-preset-minify": "0.0.0", "babel-preset-react": "^6.5.0", "babel-traverse": "^6.9.0", "babylon": "6.15.0", @@ -88,7 +86,7 @@ "through2": "^2.0.0", "tmp": "~0.0.28", "typescript": "~1.8.10", - "uglify-js": "2.8.29", + "uglify-js": "^2.8.29", "yargs": "^6.3.0" }, "devEngines": { diff --git a/packages/react/index.js b/packages/react/index.js index 42e5f9ba5c4..be66e66a687 100644 --- a/packages/react/index.js +++ b/packages/react/index.js @@ -1,9 +1,50 @@ 'use strict'; +function testMinificationUsedDCE() { + // use scoped variable for our initial test, in case + // 'top-level' mangling is not enabled. + const source = testMinificationUsedDCE.toString(); + const longVariableName = source; + if (longVariableName && source.match(/longVariableName/g).length === 3) { + // We are not minified. + // This might be a Node environment where DCE is not expected anyway. + return; + } + if (process.env.NODE_ENV === 'development') { + // We expect this method only to be called in production. + throw new Error('This is unreachable'); + } + try { + if (source.match(/toString/g).length !== 2) { + // We always look for two matches: + // The actual occurence and then the call to 'match' + // + // We know for a fact the above line exists so there should be 2 + // matches. + // Therefore the browser gave us invalid source. + return; + } + if (source.match(/unreachable/g).length === 2) { + // We always look for two matches: + // The actual occurence and then the call to 'match' + + // Dead code elimination would have stripped that branch + // because it is impossible to reach in production. + setTimeout(function() { + // Ensure it gets reported to production logging + throw new Error( + 'React is running in production mode, but dead code ' + + 'elimination has not been applied. Read how to correctly ' + + 'configure React for production: ' + + 'https://fburl.com/react-perf-use-the-production-build' + ); + }); + } + } catch (e) {} +} if (process.env.NODE_ENV === 'production') { - // TODO: actually update the build process so this works. - (require('./cjs/testMinificationUsedDCE.js'))(); + testMinificationUsedDCE(); module.exports = require('./cjs/react.production.min.js'); } else { module.exports = require('./cjs/react.development.js'); diff --git a/scripts/rollup/packaging.js b/scripts/rollup/packaging.js index 8e9d60d5d7a..ca6cbf7826f 100644 --- a/scripts/rollup/packaging.js +++ b/scripts/rollup/packaging.js @@ -30,11 +30,6 @@ const reactNativeSrcDependencies = [ 'src/renderers/native/ReactNativeTypes.js', ]; -// these files need to be copied to the react build -const reactPackageDependencies = [ - 'src/shared/utils/testMinificationUsedDCE.js', -]; - function getPackageName(name) { if (name.indexOf('/') !== -1) { return name.split('/')[0]; @@ -104,7 +99,7 @@ function copyBundleIntoNodePackage(packageName, filename, bundleType) { from = resolve(`./build/dist/${filename}`); to = `${packageDirectory}/umd/${filename}`; } - const promises = []; + // for NODE bundles we have to move the files into a cjs directory // within the package directory. we also need to set the from // to be the root build from directory @@ -115,31 +110,13 @@ function copyBundleIntoNodePackage(packageName, filename, bundleType) { fs.mkdirSync(distDirectory); } to = `${packageDirectory}/cjs/${filename}`; - if (packageName === 'react') { - // we are building the React package - let promises = []; - // we also need to copy over some specific files from src - // defined in reactPackageDependencies - for (const srcDependency of reactPackageDependencies) { - // TODO: the srcDependency needs to be processed - // before this step. - var specificFrom = resolve(srcDependency); - var specificTo = resolve(`${distDirectory}/${basename(srcDependency)}`); - promises.push( - asyncCopyTo(specificFrom, specificTo) - ); - } - } } - promises.push( - asyncCopyTo(from, to).then(() => { - // delete the old file if this is a not a UMD bundle - if (bundleType !== UMD_DEV && bundleType !== UMD_PROD) { - fs.unlinkSync(from); - } - }) - ); - return Promise.all(promises); + return asyncCopyTo(from, to).then(() => { + // delete the old file if this is a not a UMD bundle + if (bundleType !== UMD_DEV && bundleType !== UMD_PROD) { + fs.unlinkSync(from); + } + }); } else { return Promise.resolve(); } diff --git a/src/shared/utils/__tests__/testMinificationUsedDCE-test.js b/src/shared/utils/__tests__/testMinificationUsedDCE-test.js deleted file mode 100644 index 29fcce2bfdb..00000000000 --- a/src/shared/utils/__tests__/testMinificationUsedDCE-test.js +++ /dev/null @@ -1,144 +0,0 @@ -/** - * Copyright 2013-present, Facebook, Inc. - * All rights reserved. - * - * This source code is licensed under the BSD-style license found in the - * LICENSE file in the root directory of this source tree. An additional grant - * of patent rights can be found in the PATENTS file in the same directory. - * - * @emails react-core - */ -'use strict'; - -const babel = require('babel-core'); -const UglifyJs = require('uglify-js'); -let testMinificationUsedDCE; - -describe('in the production environment', () => { - let oldProcess; - beforeEach(() => { - __DEV__ = false; - - // Mutating process.env.NODE_ENV would cause our babel plugins to do the - // wrong thing. If you change this, make sure to test with jest --no-cache. - oldProcess = process; - global.process = { - ...process, - env: {...process.env, NODE_ENV: 'production'}, - }; - - jest.resetModules(); - testMinificationUsedDCE = require('testMinificationUsedDCE'); - }); - - afterEach(() => { - __DEV__ = true; - global.process = oldProcess; - }); - - describe('when not minified', () => { - it('should not throw', () => { - expect(() => { - testMinificationUsedDCE(); - jest.runAllTimers(); - }).not.toThrow(); - }); - }); - - describe('with uglifyjs', () => { - describe('when envified first, then minification with *no* DCE', () => { - it('should throw', () => { - // envify: change `process.env.NODE_ENV` to `"production"` - const rawCode = testMinificationUsedDCE - .toString() - .replace(/process\.env\.NODE_ENV/, '"production"'); - const code = {'file.js': rawCode}; - const options = {fromString: true, parse: {dead_code: false}}; - const result = UglifyJs.minify(code, options); - const minifiedWithNoDCE = () => eval('(' + result.code + ')()'); - expect(() => { - minifiedWithNoDCE(); - jest.runAllTimers(); - }).toThrow( - 'React is running in production mode, but dead code elimination ' + - 'has not been applied.', - ); - }); - }); - - describe('when envified first, then minified and *yes* successful DCE', () => { - it('should not throw', () => { - // envify: change `process.env.NODE_ENV` to `"production"` - const rawCode = testMinificationUsedDCE - .toString() - .replace(/process\.env\.NODE_ENV/g, '"production"'); - const code = {'file.js': rawCode}; - const options = {fromString: true, parse: {dead_code: true}}; - const result = UglifyJs.minify(code, options); - const minifiedWithNoDCE = () => eval('(' + result.code + ')()'); - expect(() => { - minifiedWithNoDCE(); - jest.runAllTimers(); - }).not.toThrow(); - }); - }); - - describe('when minified first with *unsuccessful* DCE, then envified', () => { - it('should throw', () => { - const code = {'file.js': testMinificationUsedDCE.toString()}; - const options = {fromString: true, parse: {dead_code: true}}; - const result = UglifyJs.minify(code, options); - // late envify: change `process.env.NODE_ENV` to `"production"` - const resultCode = result.code.replace( - /process\.env\.NODE_ENV/g, - '"production"', - ); - const minifiedWithNoDCE = () => eval('(' + resultCode + ')()'); - expect(() => { - minifiedWithNoDCE(); - jest.runAllTimers(); - }).toThrow( - 'React is running in production mode, but dead code elimination ' + - 'has not been applied.', - ); - }); - }); - }); - - describe('when minified with babel/minify with *no* DCE', () => { - xit('should throw', () => { - const babelOpts = { - presets: ['babel-preset-minify'], - }; - // TODO: Why is this not actually minifying the code???? - const minifiedWithNoDCE = () => { - eval( - babel.transform(testMinificationUsedDCE.toString(), babelOpts).code, - ); - }; - expect(() => { - minifiedWithNoDCE(); - jest.runAllTimers(); - }).toThrow(); - }); - }); - - describe('when minified with babel/minify with DCE', () => { - xit('should not throw', () => { - const babelOpts = { - plugins: ['minify-dead-code-elimination'], - presets: ['babel-preset-minify'], - }; - // TODO: Why is this not actually minifying the code???? - const minifiedWithDCE = () => { - eval( - babel.transform(testMinificationUsedDCE.toString(), babelOpts).code, - ); - }; - expect(() => { - testMinificationUsedDCE(); - jest.runAllTimers(); - }).not.toThrow(); - }); - }); -}); diff --git a/src/shared/utils/testMinificationUsedDCE.js b/src/shared/utils/testMinificationUsedDCE.js deleted file mode 100644 index 344a612338a..00000000000 --- a/src/shared/utils/testMinificationUsedDCE.js +++ /dev/null @@ -1,59 +0,0 @@ -/** - * Copyright 2014-2015, Facebook, Inc. - * All rights reserved. - * - * This source code is licensed under the BSD-style license found in the - * LICENSE file in the root directory of this source tree. An additional grant - * of patent rights can be found in the PATENTS file in the same directory. - * - * @providesModule testMinificationUsedDCE - */ - -'use strict'; - -function testMinificationUsedDCE() { - if (process.env.NODE_ENV === 'production') { - // use scoped variable for our initial test, in case - // 'top-level' mangling is not enabled. - const source = testMinificationUsedDCE.toString(); - const longVariableName = source; - if (longVariableName && source.match(/longVariableName/g).length === 3) { - // We are not minified. - // This might be a Node environment where DCE is not expected anyway. - return; - } - if (process.env.NODE_ENV === 'development') { - // We expect this method only to be called in production. - throw new Error('This is unreachable'); - } - try { - if (source.match(/toString/g).length !== 2) { - // We always look for two matches: - // The actual occurence and then the call to 'match' - // - // We know for a fact the above line exists so there should be 2 - // matches. - // Therefore the browser gave us invalid source. - return; - } - if (source.match(/unreachable/g).length === 2) { - // We always look for two matches: - // The actual occurence and then the call to 'match' - - // Dead code elimination would have stripped that branch - // because it is impossible to reach in production. - setTimeout(function() { - // Ensure it gets reported to production logging - throw new Error( - 'React is running in production mode, but dead code ' + - 'elimination has not been applied. Read how to correctly ' + - 'configure React for production: ' + - 'https://fburl.com/react-perf-use-the-production-build', - ); - }); - } - } catch (e) {} - } -} - -module.exports = testMinificationUsedDCE; From 0eecddc93b9451cd15e8156d5d4d952448a0cc66 Mon Sep 17 00:00:00 2001 From: Flarnie Marchan Date: Wed, 16 Aug 2017 07:55:17 -0700 Subject: [PATCH 11/11] Revert unintentional changes to dependency versions, variable placing **what is the change?:** - We had updated and added some dependencies, but ended up reverting this in a previous commit. The `yarn.lock` and `package.json` needed updated still though. - There is a call to `Function.toString` which we wanted to wrap in a `try/catch` block. **why make this change?:** To remove unrelated dependency changes and increase the safety of the `Function.toString` call. **test plan:** Manual testing again **issue:** https://github.com/facebook/react/issues/9589 --- package.json | 2 +- packages/react/index.js | 18 ++++----- scripts/rollup/packaging.js | 1 - yarn.lock | 77 +++++++++---------------------------- 4 files changed, 29 insertions(+), 69 deletions(-) diff --git a/package.json b/package.json index 69e8fec2e56..90511293b6a 100644 --- a/package.json +++ b/package.json @@ -86,7 +86,7 @@ "through2": "^2.0.0", "tmp": "~0.0.28", "typescript": "~1.8.10", - "uglify-js": "^2.8.29", + "uglify-js": "^2.5.0", "yargs": "^6.3.0" }, "devEngines": { diff --git a/packages/react/index.js b/packages/react/index.js index be66e66a687..8d1da5d1737 100644 --- a/packages/react/index.js +++ b/packages/react/index.js @@ -1,20 +1,20 @@ 'use strict'; function testMinificationUsedDCE() { - // use scoped variable for our initial test, in case - // 'top-level' mangling is not enabled. - const source = testMinificationUsedDCE.toString(); - const longVariableName = source; - if (longVariableName && source.match(/longVariableName/g).length === 3) { - // We are not minified. - // This might be a Node environment where DCE is not expected anyway. - return; - } if (process.env.NODE_ENV === 'development') { // We expect this method only to be called in production. throw new Error('This is unreachable'); } try { + // use scoped variable for our initial test, in case + // 'top-level' mangling is not enabled. + const source = testMinificationUsedDCE.toString(); + const longVariableName = true; + if (longVariableName && source.match(/longVariableName/g).length === 3) { + // We are not minified. + // This might be a Node environment where DCE is not expected anyway. + return; + } if (source.match(/toString/g).length !== 2) { // We always look for two matches: // The actual occurence and then the call to 'match' diff --git a/scripts/rollup/packaging.js b/scripts/rollup/packaging.js index ca6cbf7826f..e5cf6f7eae6 100644 --- a/scripts/rollup/packaging.js +++ b/scripts/rollup/packaging.js @@ -99,7 +99,6 @@ function copyBundleIntoNodePackage(packageName, filename, bundleType) { from = resolve(`./build/dist/${filename}`); to = `${packageDirectory}/umd/${filename}`; } - // for NODE bundles we have to move the files into a cjs directory // within the package directory. we also need to set the from // to be the root build from directory diff --git a/yarn.lock b/yarn.lock index 6726c50acd0..43a4ec685dd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -417,10 +417,6 @@ babel-helper-hoist-variables@^6.24.1: babel-runtime "^6.22.0" babel-types "^6.24.1" -babel-helper-mark-eval-scopes@^0.1.1: - version "0.1.1" - resolved "https://registry.yarnpkg.com/babel-helper-mark-eval-scopes/-/babel-helper-mark-eval-scopes-0.1.1.tgz#4554345edf9f2549427bd2098e530253f8af2992" - babel-helper-optimise-call-expression@^6.24.1: version "6.24.1" resolved "https://registry.yarnpkg.com/babel-helper-optimise-call-expression/-/babel-helper-optimise-call-expression-6.24.1.tgz#f7a13427ba9f73f8f4fa993c54a97882d1244257" @@ -438,10 +434,6 @@ babel-helper-remap-async-to-generator@^6.24.1: babel-traverse "^6.24.1" babel-types "^6.24.1" -babel-helper-remove-or-void@^0.1.1: - version "0.1.1" - resolved "https://registry.yarnpkg.com/babel-helper-remove-or-void/-/babel-helper-remove-or-void-0.1.1.tgz#9d7e1856dc6fafcb41b283a416730dc1844f66d7" - babel-helper-replace-supers@^6.24.1: version "6.24.1" resolved "https://registry.yarnpkg.com/babel-helper-replace-supers/-/babel-helper-replace-supers-6.24.1.tgz#bf6dbfe43938d17369a213ca8a8bf74b6a90ab1a" @@ -522,14 +514,6 @@ babel-plugin-member-expression-literals@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/babel-plugin-member-expression-literals/-/babel-plugin-member-expression-literals-1.0.1.tgz#cc5edb0faa8dc927170e74d6d1c02440021624d3" -babel-plugin-minify-dead-code-elimination@^0.1.7: - version "0.1.7" - resolved "https://registry.yarnpkg.com/babel-plugin-minify-dead-code-elimination/-/babel-plugin-minify-dead-code-elimination-0.1.7.tgz#774f536f347b98393a27baa717872968813c342c" - dependencies: - babel-helper-mark-eval-scopes "^0.1.1" - babel-helper-remove-or-void "^0.1.1" - lodash.some "^4.6.0" - babel-plugin-property-literals@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/babel-plugin-property-literals/-/babel-plugin-property-literals-1.0.1.tgz#0252301900192980b1c118efea48ce93aab83336" @@ -727,14 +711,6 @@ babel-plugin-transform-flow-strip-types@^6.22.0: babel-plugin-syntax-flow "^6.18.0" babel-runtime "^6.22.0" -babel-plugin-transform-member-expression-literals@^6.8.0: - version "6.8.4" - resolved "https://registry.yarnpkg.com/babel-plugin-transform-member-expression-literals/-/babel-plugin-transform-member-expression-literals-6.8.4.tgz#05679bc40596b91293401959aa1620ab1b2be437" - -babel-plugin-transform-merge-sibling-variables@^6.8.0: - version "6.8.5" - resolved "https://registry.yarnpkg.com/babel-plugin-transform-merge-sibling-variables/-/babel-plugin-transform-merge-sibling-variables-6.8.5.tgz#03abdf107c61241913eb268ddede6d5bc541862c" - babel-plugin-transform-object-rest-spread@^6.6.5: version "6.23.0" resolved "https://registry.yarnpkg.com/babel-plugin-transform-object-rest-spread/-/babel-plugin-transform-object-rest-spread-6.23.0.tgz#875d6bc9be761c58a2ae3feee5dc4895d8c7f921" @@ -742,12 +718,6 @@ babel-plugin-transform-object-rest-spread@^6.6.5: babel-plugin-syntax-object-rest-spread "^6.8.0" babel-runtime "^6.22.0" -babel-plugin-transform-property-literals@^6.8.0: - version "6.8.4" - resolved "https://registry.yarnpkg.com/babel-plugin-transform-property-literals/-/babel-plugin-transform-property-literals-6.8.4.tgz#6ad311110b80a192a56efb5ddf4fe3ca6f7a61da" - dependencies: - esutils "^2.0.2" - babel-plugin-transform-react-display-name@^6.23.0: version "6.23.0" resolved "https://registry.yarnpkg.com/babel-plugin-transform-react-display-name/-/babel-plugin-transform-react-display-name-6.23.0.tgz#4398910c358441dc4cef18787264d0412ed36b37" @@ -813,14 +783,6 @@ babel-preset-jest@20.1.0-delta.1: dependencies: babel-plugin-jest-hoist "20.1.0-delta.1" -babel-preset-minify@0.0.0: - version "0.0.0" - resolved "https://registry.yarnpkg.com/babel-preset-minify/-/babel-preset-minify-0.0.0.tgz#ccede0e043f80c488d65d8fe7b3ec6c2c68d284a" - dependencies: - babel-plugin-transform-member-expression-literals "^6.8.0" - babel-plugin-transform-merge-sibling-variables "^6.8.0" - babel-plugin-transform-property-literals "^6.8.0" - babel-preset-react@^6.5.0: version "6.24.1" resolved "https://registry.yarnpkg.com/babel-preset-react/-/babel-preset-react-6.24.1.tgz#ba69dfaea45fc3ec639b6a4ecea6e17702c91380" @@ -2116,7 +2078,23 @@ google-closure-compiler-js@>20170000: vinyl "^2.0.1" webpack-core "^0.6.8" -graceful-fs@^4.1.11, graceful-fs@^4.1.2, graceful-fs@^4.1.4: +got@^6.7.1: + version "6.7.1" + resolved "https://registry.yarnpkg.com/got/-/got-6.7.1.tgz#240cd05785a9a18e561dc1b44b41c763ef1e8db0" + dependencies: + create-error-class "^3.0.0" + duplexer3 "^0.1.4" + get-stream "^3.0.0" + is-redirect "^1.0.0" + is-retry-allowed "^1.0.0" + is-stream "^1.0.0" + lowercase-keys "^1.0.0" + safe-buffer "^5.0.1" + timed-out "^4.0.0" + unzip-response "^2.0.1" + url-parse-lax "^1.0.0" + +graceful-fs@^4.1.11, graceful-fs@^4.1.2, graceful-fs@^4.1.4, graceful-fs@^4.1.6, graceful-fs@^4.1.9: version "4.1.11" resolved "https://registry.yarnpkg.com/graceful-fs/-/graceful-fs-4.1.11.tgz#0e8bdfe4d1ddb8854d64e04ea7c00e2a026e5658" @@ -3106,10 +3084,6 @@ lodash.restparam@^3.0.0: version "3.6.1" resolved "https://registry.yarnpkg.com/lodash.restparam/-/lodash.restparam-3.6.1.tgz#936a4e309ef330a7645ed4145986c85ae5b20805" -lodash.some@^4.6.0: - version "4.6.0" - resolved "https://registry.yarnpkg.com/lodash.some/-/lodash.some-4.6.0.tgz#1bb9f314ef6b8baded13b549169b2a945eb68e4d" - lodash.template@^3.0.0: version "3.6.2" resolved "https://registry.yarnpkg.com/lodash.template/-/lodash.template-3.6.2.tgz#f8cdecc6169a255be9098ae8b0c53d378931d14f" @@ -3844,7 +3818,7 @@ replace-ext@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/replace-ext/-/replace-ext-1.0.0.tgz#de63128373fcbf7c3ccfa4de5a480c45a67958eb" -request@2.79.0: +request@2, request@2.79.0: version "2.79.0" resolved "https://registry.yarnpkg.com/request/-/request-2.79.0.tgz#4dfe5bf6be8b8cdc37fcf93e04b65577722710de" dependencies: @@ -4457,15 +4431,11 @@ typedarray@~0.0.5: version "0.0.6" resolved "https://registry.yarnpkg.com/typedarray/-/typedarray-0.0.6.tgz#867ac74e3864187b1d3d47d996a78ec5c8830777" -typescript@~1.8.10: - version "1.8.10" - resolved "https://registry.yarnpkg.com/typescript/-/typescript-1.8.10.tgz#b475d6e0dff0bf50f296e5ca6ef9fbb5c7320f1e" - ua-parser-js@^0.7.9: version "0.7.12" resolved "https://registry.yarnpkg.com/ua-parser-js/-/ua-parser-js-0.7.12.tgz#04c81a99bdd5dc52263ea29d24c6bf8d4818a4bb" -uglify-js@^2.6, uglify-js@^2.6.1: +uglify-js@^2.5.0, uglify-js@^2.6, uglify-js@^2.6.1: version "2.8.22" resolved "https://registry.yarnpkg.com/uglify-js/-/uglify-js-2.8.22.tgz#d54934778a8da14903fa29a326fb24c0ab51a1a0" dependencies: @@ -4474,15 +4444,6 @@ uglify-js@^2.6, uglify-js@^2.6.1: optionalDependencies: uglify-to-browserify "~1.0.0" -uglify-js@^2.8.29: - version "2.8.29" - resolved "https://registry.yarnpkg.com/uglify-js/-/uglify-js-2.8.29.tgz#29c5733148057bb4e1f75df35b7a9cb72e6a59dd" - dependencies: - source-map "~0.5.1" - yargs "~3.10.0" - optionalDependencies: - uglify-to-browserify "~1.0.0" - uglify-to-browserify@~1.0.0: version "1.0.2" resolved "https://registry.yarnpkg.com/uglify-to-browserify/-/uglify-to-browserify-1.0.2.tgz#6e0924d6bda6b5afe349e39a6d632850a0f882b7"