Skip to content

Commit

Permalink
Merge pull request #1070 from stealjs/amd-shake
Browse files Browse the repository at this point in the history
Enable tree-shaking ES modules only imported by non-ES modules
  • Loading branch information
matthewp committed Aug 30, 2018
2 parents 1397fe6 + aa98380 commit 461227c
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 9 deletions.
21 changes: 15 additions & 6 deletions lib/graph/treeshake.js
Expand Up @@ -112,29 +112,38 @@ function loadFromGraph(getNode) {
}
}

let code = '';

if(node) {
// Add deps as regular imports
for(let i = 0; i < node.load.metadata.deps.length; i++) {
let specifier = node.load.metadata.deps[i];
code += `import * as dep${i} from "${specifier}";\n`;
code += `global.someFunction(dep${i}.default);\n`;
}
}

// Expose named exports so that dependant modules will tree-shake properly.
if(needToExport.size) {
let code = '';
for(let exp of needToExport) {
if(exp === "default") {
code += "export default {};\n";
} else {
code += `export let ${exp} = {};\n`;
}
}

return code;
} else {
// Prevent tree shaking modules that are side-effectual like CSS
if(sideEffectualModule(node)) {
return `
code += `
export function one() {window.ONE = {}};
one();
`.trim();
} else {
code += "export default {}";
}

return "export default {}";
}
return code;
}

return source.node(node);
Expand Down
8 changes: 8 additions & 0 deletions test/tree_shaking_test.js
Expand Up @@ -126,6 +126,14 @@ describe("Tree-shaking", function(){
assert.equal(connect.didFail, true, "marked as failed");
});
});

describe("ES modules imported by CommonJS modules", function() {
it("Tree-shakes their ES module dependencies", function() {
let mod = app.dep5dep5AnotherESModule;
assert.equal(typeof mod.mainThing, "function", "Kept the used export");
assert.equal(typeof mod.another, "undefined", "Removed the unused export");
});
});
});

describe("treeShaking: false", function(){
Expand Down
9 changes: 6 additions & 3 deletions test/treeshake/basics/main.js
Expand Up @@ -53,7 +53,8 @@ export default function(){
steal.import("dep4/other"),
steal.import("dep4/and-another"),
steal.import("~/from-exports"),
shouldFail
shouldFail,
steal.import("dep5/another-es-module")
]);

return p
Expand All @@ -64,7 +65,8 @@ export default function(){
dep4Other,
dep4AndAnother,
fromExports,
canConnect
canConnect,
dep5dep5AnotherESModule
]) => {
return {
anon,
Expand All @@ -75,7 +77,8 @@ export default function(){
dep4AndAnother,
fromExports,
dep5,
canConnect
canConnect,
dep5dep5AnotherESModule
};
});
};
8 changes: 8 additions & 0 deletions test/treeshake/basics/node_modules/dep5/another-es-module.js

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

5 changes: 5 additions & 0 deletions test/treeshake/basics/node_modules/dep5/es-module.js

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

2 changes: 2 additions & 0 deletions test/treeshake/basics/node_modules/dep5/main.js

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

0 comments on commit 461227c

Please sign in to comment.