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.

Sign up for GitHub

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

Throw error to warn of mistaken loading of prod + dev React bundles #10446

Merged
merged 11 commits into from
Aug 16, 2017
Merged

Throw error to warn of mistaken loading of prod + dev React bundles #10446

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
8e280c8
Throw error to warn of mistaken loading of prod + dev React bundles
flarnie Aug 11, 2017
441d661
Remove extra 'prod' env. check and add link to docs in error message
flarnie Aug 12, 2017
05be49a
WIP fix and test 'testMinificationUsedDCE' method
flarnie Aug 13, 2017
8fab885
Add babeli and uglifyjs as dev-only dependencies
flarnie Aug 13, 2017
9a73704
Fix typo and add 'uglifyjs' tests
flarnie Aug 14, 2017
5204310
Run prettier
flarnie Aug 14, 2017
26ce0ac
var -> const/let
flarnie Aug 14, 2017
498f8f5
Require specific version of `uglify-js`
flarnie Aug 14, 2017
ab6c1f0
Add build step copying testMinificationUsedDCE into build/packages/re…
flarnie Aug 14, 2017
8a1c36b
Inline 'testMinificationUsedDCE' and remove unit test for now
flarnie Aug 14, 2017
0eecddc
Revert unintentional changes to dependency versions, variable placing
flarnie Aug 16, 2017
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
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,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": {
Expand Down
44 changes: 44 additions & 0 deletions packages/react/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +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();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wanted this line to be inside try catch in case toString throws. I think this can happen in restrictive environments although not sure where I saw that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing - I forget why I moved it out, will put it back.

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') {
testMinificationUsedDCE();
module.exports = require('./cjs/react.production.min.js');
} else {
module.exports = require('./cjs/react.development.js');
Expand Down
1 change: 1 addition & 0 deletions scripts/rollup/packaging.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ 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
Expand Down
77 changes: 58 additions & 19 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -711,13 +727,27 @@ 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"
dependencies:
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"
Expand Down Expand Up @@ -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"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we revert changes to Yarn lock? Seems like Uglify update could also go into a separate PR with just necessary lockfile changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack - yes, both of those should be removed from this diff. Will fix that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It would still be nice to update it separately though :-) maybe it has improved in the meantime.

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"
Expand Down Expand Up @@ -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"

Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand All @@ -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"
Expand Down