Skip to content
This repository was archived by the owner on Jun 26, 2020. It is now read-only.

Commit 06935c1

Browse files
authored
Add bad dead code elimination detection for React 16 (#888)
* Add bad DCE detection for React 16 * Throw an error in setTimeout for reporting
1 parent 4acf417 commit 06935c1

File tree

12 files changed

+70
-27
lines changed

12 files changed

+70
-27
lines changed

backend/installGlobalHook.js

Lines changed: 40 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ function installGlobalHook(window: Object) {
2222
}
2323
function detectReactBuildType(renderer) {
2424
try {
25-
var toString = Function.prototype.toString;
2625
if (typeof renderer.version === 'string') {
2726
// React DOM Fiber (16+)
2827
if (renderer.bundleType > 0) {
@@ -31,29 +30,17 @@ function installGlobalHook(window: Object) {
3130
// but might add 2 (PROFILE) in the future.
3231
return 'development';
3332
}
34-
// The above should cover envification, but we should still make sure
35-
// that the bundle code has been uglified.
36-
var findFiberCode = toString.call(renderer.findFiberByHostInstance);
37-
// Filter out bad results (if that is even possible):
38-
if (findFiberCode.indexOf('function') !== 0) {
39-
// Hope for the best if we're not sure.
40-
return 'production';
41-
}
42-
43-
// By now we know that it's envified--but what if it's not minified?
44-
// This can be bad too, as it means DEV code is still there.
45-
46-
// FIXME: this is fragile!
47-
// We should replace this check with check on a specially passed
48-
// function. This also doesn't detect lack of dead code elimination
49-
// (although this is not a problem since flat bundles).
50-
if (findFiberCode.indexOf('getClosestInstanceFromNode') !== -1) {
51-
return 'unminified';
52-
}
5333

54-
// We're good.
34+
// React 16 uses flat bundles. If we report the bundle as production
35+
// version, it means we also minified and envified it ourselves.
5536
return 'production';
37+
// Note: There is still a risk that the CommonJS entry point has not
38+
// been envified or uglified. In this case the user would have *both*
39+
// development and production bundle, but only the prod one would run.
40+
// This would be really bad. We have a separate check for this because
41+
// it happens *outside* of the renderer injection. See `checkDCE` below.
5642
}
43+
var toString = Function.prototype.toString;
5744
if (renderer.Mount && renderer.Mount._renderNewRootComponent) {
5845
// React DOM Stack
5946
var renderRootCode = toString.call(renderer.Mount._renderNewRootComponent);
@@ -141,14 +128,45 @@ function installGlobalHook(window: Object) {
141128
}
142129
return 'production';
143130
}
131+
132+
let hasDetectedBadDCE = false;
133+
144134
const hook = ({
145135
// Shared between Stack and Fiber:
146136
_renderers: {},
147137
helpers: {},
138+
checkDCE: function(fn) {
139+
// This runs for production versions of React.
140+
// Needs to be super safe.
141+
try {
142+
var toString = Function.prototype.toString;
143+
var code = toString.call(fn);
144+
// This is a string embedded in the passed function under DEV-only
145+
// condition. However the function executes only in PROD. Therefore,
146+
// if we see it, dead code elimination did not work.
147+
if (code.indexOf('^_^') > -1) {
148+
// Remember to report during next injection.
149+
hasDetectedBadDCE = true;
150+
// Bonus: throw an exception hoping that it gets picked up by
151+
// a reporting system. Not synchronously so that it doesn't break the
152+
// calling code.
153+
setTimeout(function() {
154+
throw new Error(
155+
'React is running in production mode, but dead code ' +
156+
'elimination has not been applied. Read how to correctly ' +
157+
'configure React for production: ' +
158+
'https://fburl.com/react-perf-use-the-production-build'
159+
);
160+
});
161+
}
162+
} catch (err) { }
163+
},
148164
inject: function(renderer) {
149165
var id = Math.random().toString(16).slice(2);
150166
hook._renderers[id] = renderer;
151-
var reactBuildType = detectReactBuildType(renderer);
167+
var reactBuildType = hasDetectedBadDCE ?
168+
'deadcode' :
169+
detectReactBuildType(renderer);
152170
hook.emit('renderer', {id, renderer, reactBuildType});
153171
return id;
154172
},
5.08 KB
Loading
638 Bytes
Loading
1.29 KB
Loading
1.95 KB
Loading
Lines changed: 1 addition & 0 deletions
Loading
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<script src="shared.js"></script>
2+
<style>
3+
html, body {
4+
font-size: 14px;
5+
min-width: 460px;
6+
min-height: 133px;
7+
}
8+
</style>
9+
<p>
10+
<b>This page includes an extra development build of React. &#x1f6a7;</b>
11+
</p>
12+
<p>
13+
The React build on this page includes both development and production versions because dead code elimination has not been applied correctly.
14+
<br />
15+
<br />
16+
This makes its size larger, and causes React to run slower.
17+
<br />
18+
<br />
19+
Make sure to <a href="https://facebook.github.io/react/docs/optimizing-performance.html#use-the-production-build">set up dead code elimination</a> before deployment.
20+
</p>
21+
<hr />
22+
<p>
23+
Open the developer tools, and the React tab will appear to the right.
24+
</p>

shells/webextension/popups/development.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
<style>
33
html, body {
44
font-size: 14px;
5-
min-width: 430px;
5+
min-width: 460px;
66
min-height: 101px;
77
}
88
</style>

shells/webextension/popups/disabled.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
<style>
33
html, body {
44
font-size: 14px;
5-
min-width: 380px;
5+
min-width: 410px;
66
min-height: 33px;
77
}
88
</style>

shells/webextension/popups/outdated.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
<style>
33
html, body {
44
font-size: 14px;
5-
min-width: 430px;
5+
min-width: 460px;
66
min-height: 117px;
77
}
88
</style>

0 commit comments

Comments
 (0)