Skip to content

Commit

Permalink
Avoid unnecessary facade dependency inlining (#3552)
Browse files Browse the repository at this point in the history
* implement chunk pruning

* remove unused import

* seems it can

* remove unnecessary phase

* remove unused test files

* Fix dependencies in facade creation

* Respect configured entry order and creates facades from later entries

Co-authored-by: Lukas Taegert-Atkinson <lukas.taegert-atkinson@tngtech.com>
  • Loading branch information
guybedford and lukastaegert committed May 13, 2020
1 parent 3cbcc8d commit 07e0b20
Show file tree
Hide file tree
Showing 59 changed files with 371 additions and 28 deletions.
7 changes: 6 additions & 1 deletion src/Chunk.ts
Expand Up @@ -126,7 +126,12 @@ export default class Chunk {
if (!facadedModule.facadeChunk) {
facadedModule.facadeChunk = chunk;
}
chunk.dependencies.add(facadedModule.chunk!);
for (const dependency of facadedModule.getDependenciesToBeIncluded()) {
chunk.dependencies.add(dependency instanceof Module ? dependency.chunk! : dependency);
}
if (!chunk.dependencies.has(facadedModule.chunk!) && facadedModule.hasEffects()) {
chunk.dependencies.add(facadedModule.chunk!);
}
chunk.facadeModule = facadedModule;
chunk.strictFacade = true;
return chunk;
Expand Down
11 changes: 7 additions & 4 deletions src/Module.ts
Expand Up @@ -368,10 +368,7 @@ export default class Module {
const possibleDependencies = new Set(this.dependencies);
for (const dependency of possibleDependencies) {
if (!dependency.moduleSideEffects || relevantDependencies.has(dependency)) continue;
if (
dependency instanceof ExternalModule ||
(dependency.ast.included && dependency.ast.hasEffects(createHasEffectsContext()))
) {
if (dependency instanceof ExternalModule || dependency.hasEffects()) {
relevantDependencies.add(dependency);
} else {
for (const transitiveDependency of dependency.dependencies) {
Expand Down Expand Up @@ -532,6 +529,12 @@ export default class Module {
return null as any;
}

hasEffects() {
return (
this.moduleSideEffects && this.ast.included && this.ast.hasEffects(createHasEffectsContext())
);
}

include(): void {
const context = createInclusionContext();
if (this.ast.shouldBeIncluded(context)) this.ast.include(context, false);
Expand Down
35 changes: 17 additions & 18 deletions src/ModuleLoader.ts
Expand Up @@ -142,32 +142,31 @@ export class ModuleLoader {
this.nextEntryModuleIndex += unresolvedEntryModules.length;
const loadNewEntryModulesPromise = Promise.all(
unresolvedEntryModules.map(
async ({ fileName, id, name, importer }): Promise<Module> => {
const module = await this.loadEntryModule(id, true, importer);
if (fileName !== null) {
module.chunkFileNames.add(fileName);
} else if (name !== null) {
if (module.chunkName === null) {
module.chunkName = name;
}
if (isUserDefined) {
module.userChunkNames.add(name);
}
}
return module;
}
({ id, importer }): Promise<Module> => this.loadEntryModule(id, true, importer)
)
).then(entryModules => {
let moduleIndex = firstEntryModuleIndex;
for (const entryModule of entryModules) {
for (let index = 0; index < entryModules.length; index++) {
const { fileName, name } = unresolvedEntryModules[index];
const entryModule = entryModules[index];
entryModule.isUserDefinedEntryPoint = entryModule.isUserDefinedEntryPoint || isUserDefined;
const existingIndexModule = this.indexedEntryModules.find(
if (fileName !== null) {
entryModule.chunkFileNames.add(fileName);
} else if (name !== null) {
if (entryModule.chunkName === null) {
entryModule.chunkName = name;
}
if (isUserDefined) {
entryModule.userChunkNames.add(name);
}
}
const existingIndexedModule = this.indexedEntryModules.find(
indexedModule => indexedModule.module.id === entryModule.id
);
if (!existingIndexModule) {
if (!existingIndexedModule) {
this.indexedEntryModules.push({ module: entryModule, index: moduleIndex });
} else {
existingIndexModule.index = Math.min(existingIndexModule.index, moduleIndex);
existingIndexedModule.index = Math.min(existingIndexedModule.index, moduleIndex);
}
moduleIndex++;
}
Expand Down
13 changes: 13 additions & 0 deletions test/chunking-form/samples/entry-aliases/_config.js
@@ -0,0 +1,13 @@
module.exports = {
description: 'alias module dependency inlining',
options: {
input: {
'main1.js': 'main1.js',
'main1-alias.js': 'main1.js',
'main2.js': 'main2.js'
},
output: {
entryFileNames: '[name]'
}
}
};
@@ -0,0 +1,7 @@
define(['exports'], function (exports) { 'use strict';

var name = 'name';

exports.name = name;

});
@@ -0,0 +1,9 @@
define(['exports', './generated-dep'], function (exports, dep) { 'use strict';



exports.name = dep.name;

Object.defineProperty(exports, '__esModule', { value: true });

});
@@ -0,0 +1,9 @@
define(['exports', './generated-dep'], function (exports, dep) { 'use strict';



exports.name = dep.name;

Object.defineProperty(exports, '__esModule', { value: true });

});
@@ -0,0 +1,5 @@
define(['./generated-dep'], function (dep) { 'use strict';

console.log(dep.name);

});
@@ -0,0 +1,5 @@
'use strict';

var name = 'name';

exports.name = name;
@@ -0,0 +1,9 @@
'use strict';

Object.defineProperty(exports, '__esModule', { value: true });

var dep = require('./generated-dep.js');



exports.name = dep.name;
@@ -0,0 +1,9 @@
'use strict';

Object.defineProperty(exports, '__esModule', { value: true });

var dep = require('./generated-dep.js');



exports.name = dep.name;
@@ -0,0 +1,5 @@
'use strict';

var dep = require('./generated-dep.js');

console.log(dep.name);
@@ -0,0 +1,3 @@
var name = 'name';

export { name as n };
@@ -0,0 +1 @@
export { n as name } from './generated-dep.js';
@@ -0,0 +1 @@
export { n as name } from './generated-dep.js';
@@ -0,0 +1,3 @@
import { n as name } from './generated-dep.js';

console.log(name);
@@ -0,0 +1,10 @@
System.register([], function (exports) {
'use strict';
return {
execute: function () {

var name = exports('n', 'name');

}
};
});
@@ -0,0 +1,13 @@
System.register(['./generated-dep.js'], function (exports) {
'use strict';
return {
setters: [function (module) {
exports('name', module.n);
}],
execute: function () {



}
};
});
13 changes: 13 additions & 0 deletions test/chunking-form/samples/entry-aliases/_expected/system/main1.js
@@ -0,0 +1,13 @@
System.register(['./generated-dep.js'], function (exports) {
'use strict';
return {
setters: [function (module) {
exports('name', module.n);
}],
execute: function () {



}
};
});
14 changes: 14 additions & 0 deletions test/chunking-form/samples/entry-aliases/_expected/system/main2.js
@@ -0,0 +1,14 @@
System.register(['./generated-dep.js'], function () {
'use strict';
var name;
return {
setters: [function (module) {
name = module.n;
}],
execute: function () {

console.log(name);

}
};
});
1 change: 1 addition & 0 deletions test/chunking-form/samples/entry-aliases/dep.js
@@ -0,0 +1 @@
export var name = 'name';
1 change: 1 addition & 0 deletions test/chunking-form/samples/entry-aliases/main1.js
@@ -0,0 +1 @@
export { name } from './dep.js';
2 changes: 2 additions & 0 deletions test/chunking-form/samples/entry-aliases/main2.js
@@ -0,0 +1,2 @@
import { name } from './dep.js';
console.log(name);
@@ -1,4 +1,4 @@
define(['exports', './m2', './generated-m1'], function (exports, m2, m1) { 'use strict';
define(['exports', './m2'], function (exports, m2) { 'use strict';



Expand Down
Expand Up @@ -3,7 +3,6 @@
Object.defineProperty(exports, '__esModule', { value: true });

var m2 = require('./m2.js');
require('./generated-m1.js');



Expand Down
@@ -1,2 +1 @@
export { default as m2 } from './m2.js';
import './generated-m1.js';
@@ -1,9 +1,9 @@
System.register(['./m2.js', './generated-m1.js'], function (exports) {
System.register(['./m2.js'], function (exports) {
'use strict';
return {
setters: [function (module) {
exports('m2', module.default);
}, function () {}],
}],
execute: function () {


Expand Down
10 changes: 10 additions & 0 deletions test/form/samples/prune-pure-unused-import-array/_config.js
@@ -0,0 +1,10 @@
module.exports = {
description: 'prunes pure unused external imports ([#1352])',
options: {
external: ['external', 'other'],
treeshake: { moduleSideEffects: ['other'] },
output: {
globals: { other: 'other' }
}
}
};
@@ -0,0 +1,5 @@
define(['other'], function (other) { 'use strict';



});
@@ -0,0 +1,4 @@
'use strict';

require('other');

@@ -0,0 +1 @@
import 'other';
@@ -0,0 +1,6 @@
(function (other) {
'use strict';



}(other));
@@ -0,0 +1,11 @@
System.register(['other'], function () {
'use strict';
return {
setters: [function () {}],
execute: function () {



}
};
});
@@ -0,0 +1,9 @@
(function (global, factory) {
typeof exports === 'object' && typeof module !== 'undefined' ? factory(require('other')) :
typeof define === 'function' && define.amd ? define(['other'], factory) :
(global = global || self, factory(global.other));
}(this, (function (other) { 'use strict';



})));
7 changes: 7 additions & 0 deletions test/form/samples/prune-pure-unused-import-array/main.js
@@ -0,0 +1,7 @@
import { unused } from 'external';
import { notused } from 'other';

function alsoUnused () {
unused();
notused();
}
11 changes: 11 additions & 0 deletions test/form/samples/prune-pure-unused-import-function/_config.js
@@ -0,0 +1,11 @@
module.exports = {
description: 'prunes pure unused external imports ([#1352])',
expectedWarnings: ['EMPTY_BUNDLE'],
options: {
external: ['external', 'other'],
treeshake: { moduleSideEffects: id => id !== 'external' },
output: {
globals: { other: 'other' }
}
}
};
@@ -0,0 +1,5 @@
define(['other'], function (other) { 'use strict';



});
@@ -0,0 +1,4 @@
'use strict';

require('other');

@@ -0,0 +1 @@
import 'other';
@@ -0,0 +1,6 @@
(function (other) {
'use strict';



}(other));
@@ -0,0 +1,11 @@
System.register(['other'], function () {
'use strict';
return {
setters: [function () {}],
execute: function () {



}
};
});
@@ -0,0 +1,9 @@
(function (global, factory) {
typeof exports === 'object' && typeof module !== 'undefined' ? factory(require('other')) :
typeof define === 'function' && define.amd ? define(['other'], factory) :
(global = global || self, factory(global.other));
}(this, (function (other) { 'use strict';



})));
7 changes: 7 additions & 0 deletions test/form/samples/prune-pure-unused-import-function/main.js
@@ -0,0 +1,7 @@
import { unused } from 'external';
import { notused } from 'other';

function alsoUnused () {
unused();
notused();
}

0 comments on commit 07e0b20

Please sign in to comment.