Skip to content

Commit

Permalink
WIP fix and test 'testMinificationUsedDCE' method
Browse files Browse the repository at this point in the history
**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:**
facebook#9589
  • Loading branch information
flarnie committed Aug 13, 2017
1 parent 441d661 commit 05be49a
Show file tree
Hide file tree
Showing 3 changed files with 144 additions and 31 deletions.
33 changes: 2 additions & 31 deletions packages/react/index.js
Original file line number Diff line number Diff line change
@@ -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');
Expand Down
86 changes: 86 additions & 0 deletions src/shared/utils/__tests__/testMinificationUsedDCE-test.js
Original file line number Diff line number Diff line change
@@ -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();
});
});
});
56 changes: 56 additions & 0 deletions src/shared/utils/testMinificationUsedDCE.js
Original file line number Diff line number Diff line change
@@ -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;

0 comments on commit 05be49a

Please sign in to comment.