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

Changes from 2 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
32 changes: 32 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,38 @@
'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) {
Copy link

Choose a reason for hiding this comment

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

Similar thing here, this works, but is kinda misleading.

const source = testMinification.toString(); could be replaced with const source = String(testMinification); and this check would still work.

// We know for a fact the above line exists.
// Therefore the browser gave us invalid source.
return;
}
if (source.indexOf('unreachable') !== -1) {
Copy link

Choose a reason for hiding this comment

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

This is always true because it will find its own search string, i.e.

(function foo() { return foo.toString().indexOf('any arbitrary string') !== -1; })();

so this error will be thrown in correctly minified code.

Extracting it to a variable might work:

var searchString = 'any arbitrary string';
(function foo() { return foo.toString().indexOf(searchString) !== -1; })();

but Babili will inline it, recreating the original problem.

'UNREACHABLE'.toLowerCase() appears to work for now, but probably not in the future, as Babili currently evaluates some other String.prototype methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

! Good catch! I'll update it. Maybe we can have it look for two occurrences of the search string.

// 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();
module.exports = require('./cjs/react.production.min.js');
} else {
module.exports = require('./cjs/react.development.js');
Expand Down