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.

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

Ignore noop dependencies #1166

Merged
merged 5 commits into from
May 3, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/assets/JSAsset.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@ const Asset = require('../Asset');
const babylon = require('babylon');
const insertGlobals = require('../visitors/globals');
const fsVisitor = require('../visitors/fs');
const envVisitor = require('../visitors/env');
const babel = require('../transforms/babel');
const generate = require('babel-generator').default;
const uglify = require('../transforms/uglify');
const SourceMap = require('../SourceMap');

const IMPORT_RE = /\b(?:import\b|export\b|require\s*\()/;
const ENV_RE = /\b(?:process\.env)\b/;
const GLOBAL_RE = /\b(?:process|__dirname|__filename|global|Buffer)\b/;
const FS_RE = /\breadFileSync\b/;
const SW_RE = /\bnavigator\s*\.\s*serviceWorker\s*\.\s*register\s*\(/;
Expand Down Expand Up @@ -93,6 +95,12 @@ class JSAsset extends Asset {

async pretransform() {
await babel(this);

// Inline environment variables
if (ENV_RE.test(this.contents)) {
await this.parseIfNeeded();
this.traverseFast(envVisitor);
}
}

async transform() {
Expand Down
35 changes: 34 additions & 1 deletion src/visitors/dependencies.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const types = require('babel-types');
const template = require('babel-template');
const traverse = require('babel-traverse').default;
const urlJoin = require('../utils/urlJoin');
const isURL = require('../utils/is-url');
const matchesPattern = require('./matches-pattern');
Expand Down Expand Up @@ -38,7 +39,8 @@ module.exports = {
callee.name === 'require' &&
args.length === 1 &&
types.isStringLiteral(args[0]) &&
!hasBinding(ancestors, 'require');
!hasBinding(ancestors, 'require') &&
!isInFalsyBranch(ancestors);

if (isRequire) {
let optional = ancestors.some(a => types.isTryStatement(a)) || undefined;
Expand Down Expand Up @@ -116,6 +118,37 @@ function hasBinding(node, name) {
return false;
}

function isInFalsyBranch(ancestors) {
// Check if any ancestors are if statements
return ancestors.some((node, index) => {
if (types.isIfStatement(node)) {
let res = evaluateExpression(node.test);
if (res && res.confident) {
// If the test is truthy, exclude the dep if it is in the alternate branch.
// If the test if falsy, exclude the dep if it is in the consequent branch.
let child = ancestors[index + 1];
return res.value ? child === node.alternate : child === node.consequent;
}
}
});
}

function evaluateExpression(node) {
// Wrap the node in a standalone program so we can traverse it
node = types.file(types.program([types.expressionStatement(node)]));

// Find the first expression and evaluate it.
let res = null;
traverse(node, {
Expression(path) {
res = path.evaluate();
path.stop();
}
});

return res;
}

function addDependency(asset, node, opts = {}) {
if (asset.options.target !== 'browser') {
const isRelativeImport = /^[/~.]/.test(node.value);
Expand Down
28 changes: 28 additions & 0 deletions src/visitors/env.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
const types = require('babel-types');
const matchesPattern = require('./matches-pattern');

module.exports = {
MemberExpression(node, asset) {
// Inline environment variables accessed on process.env
if (matchesPattern(node.object, 'process.env')) {
let key = types.toComputedKey(node);
if (types.isStringLiteral(key)) {
let val = types.valueToNode(process.env[key.value]);
morph(node, val);
asset.isAstDirty = true;
asset.cacheData.env[key.value] = process.env[key.value];
}
}
}
};

// replace object properties
function morph(object, newProperties) {
for (let key in object) {
delete object[key];
}

for (let key in newProperties) {
object[key] = newProperties[key];
}
}
25 changes: 0 additions & 25 deletions src/visitors/globals.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
const Path = require('path');
const types = require('babel-types');
const matchesPattern = require('./matches-pattern');

const VARS = {
process: asset => {
Expand All @@ -18,19 +17,6 @@ const VARS = {
};

module.exports = {
MemberExpression(node, asset) {
// Inline environment variables accessed on process.env
if (matchesPattern(node.object, 'process.env')) {
let key = types.toComputedKey(node);
if (types.isStringLiteral(key)) {
let val = types.valueToNode(process.env[key.value]);
morph(node, val);
asset.isAstDirty = true;
asset.cacheData.env[key.value] = process.env[key.value];
}
}
},

Identifier(node, asset, ancestors) {
let parent = ancestors[ancestors.length - 2];
if (
Expand Down Expand Up @@ -63,14 +49,3 @@ function inScope(ancestors) {

return false;
}

// replace object properties
function morph(object, newProperties) {
for (let key in object) {
delete object[key];
}

for (let key in newProperties) {
object[key] = newProperties[key];
}
}
27 changes: 27 additions & 0 deletions test/integration/falsy-dep/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
if (false) {
require('if-false-optional-dep');
}

if (false) {
globalStuff(() =>
require('if-false-optional-dep-deep')
);
}

if ('') {
require('if-falsy-optional-dep');
}

if (process.env.NODE_ENV === 'test') {
require('./true-consequent');
} else {
require('./false-alternate');
}

if (process.env.NODE_ENV !== 'test') {
require('./false-consequent');
} else {
require('./true-alternate');
}

module.exports = 2;
Empty file.
Empty file.
17 changes: 17 additions & 0 deletions test/javascript.js
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,23 @@ describe('javascript', function() {
assert.deepEqual(output, err);
});

it('should support excluding dependencies in falsy branches', async function() {
let b = await bundle(__dirname + '/integration/falsy-dep/index.js');

assertBundleTree(b, {
name: 'index.js',
assets: ['index.js', 'true-alternate.js', 'true-consequent.js'],
childBundles: [
{
type: 'map'
}
]
});

let output = run(b);
assert.equal(output, 2);
});

it('should not autoinstall if resolve failed on installed module', async function() {
let error;
try {
Expand Down