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

Support array in package.json's sideEffects property #1766

Merged
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
2 changes: 1 addition & 1 deletion src/Asset.js
Expand Up @@ -125,7 +125,7 @@ class Asset {

async getPackage() {
if (!this._package) {
this._package = await this.getConfig(['package.json']);
this._package = await this.resolver.findPackage(path.dirname(this.name));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this changed?

Copy link
Contributor Author

@ahocevar ahocevar Jul 21, 2018

Choose a reason for hiding this comment

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

Because findPackage() will add a pkgdir property to the package, which is needed later for globbing the sideEffects array entries.

}

return this._package;
Expand Down
4 changes: 2 additions & 2 deletions src/packagers/JSConcatPackager.js
Expand Up @@ -141,8 +141,8 @@ class JSConcatPackager extends Packager {
this.addedAssets.add(asset);
let {js} = asset.generated;

// If the asset's package has the sideEffects: false flag set, and there are no used
// exports marked, exclude the asset from the bundle.
// If the asset has no side effects according to the its package's sideEffects flag,
// and there are no used exports marked, exclude the asset from the bundle.
if (
asset.cacheData.sideEffects === false &&
(!asset.usedExports || asset.usedExports.size === 0)
Expand Down
24 changes: 22 additions & 2 deletions src/scope-hoisting/hoist.js
@@ -1,4 +1,6 @@
const path = require('path');
const matchesPattern = require('../visitors/matches-pattern');
const mm = require('micromatch');
const t = require('babel-types');
const template = require('babel-template');
const rename = require('./renamer');
Expand Down Expand Up @@ -28,14 +30,32 @@ const TYPEOF = {
require: 'function'
};

function hasSideEffects(asset, {sideEffects} = asset._package) {
switch (typeof sideEffects) {
case 'undefined':
return true;
case 'boolean':
return sideEffects;
case 'string':
return mm.isMatch(
path.relative(asset._package.pkgdir, asset.name),
sideEffects,
{matchBase: true}
);
case 'object':
return sideEffects.some(sideEffects =>
hasSideEffects(asset, {sideEffects})
);
}
}

module.exports = {
Program: {
enter(path, asset) {
asset.cacheData.imports = asset.cacheData.imports || Object.create(null);
asset.cacheData.exports = asset.cacheData.exports || Object.create(null);
asset.cacheData.wildcards = asset.cacheData.wildcards || [];
asset.cacheData.sideEffects =
asset._package && asset._package.sideEffects;
asset.cacheData.sideEffects = asset._package && hasSideEffects(asset);

let shouldWrap = false;
path.traverse({
Expand Down
1 change: 0 additions & 1 deletion test/graphql.js
Expand Up @@ -31,7 +31,6 @@ describe('graphql', function() {
firstName
lastName
}

`.definitions
);
});
Expand Down
3 changes: 3 additions & 0 deletions test/integration/scope-hoisting/es6/side-effects-array/a.js
@@ -0,0 +1,3 @@
import {foo} from 'bar';

output = foo(2);

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 16 additions & 0 deletions test/scope-hoisting.js
Expand Up @@ -281,6 +281,22 @@ describe('scope hoisting', function() {
assert.deepEqual(output, 'bar');
});

it('supports the package.json sideEffects flag with an array', async function() {
let b = await bundle(
__dirname + '/integration/scope-hoisting/es6/side-effects-array/a.js'
);

let calls = [];
let output = await run(b, {
sideEffect: caller => {
calls.push(caller);
}
});

assert(calls.toString() == 'foo', "side effect called for 'foo'");
assert.deepEqual(output, 4);
});

it('missing exports should be replaced with an empty object', async function() {
let b = await bundle(
__dirname + '/integration/scope-hoisting/es6/empty-module/a.js'
Expand Down