Skip to content

Commit

Permalink
Fix dependencies in facade creation
Browse files Browse the repository at this point in the history
  • Loading branch information
lukastaegert committed May 12, 2020
1 parent 622a89b commit d0312ff
Show file tree
Hide file tree
Showing 52 changed files with 264 additions and 119 deletions.
100 changes: 31 additions & 69 deletions 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 All @@ -137,7 +142,6 @@ export default class Chunk {
exportMode: 'none' | 'named' | 'default' = 'named';
facadeModule: Module | null = null;
graph: Graph;
hasSideEffects = false;
id: string | null = null;
indentString: string = undefined as any;
isDynamicEntry = false;
Expand All @@ -148,7 +152,6 @@ export default class Chunk {
};
usedModules: Module[] = undefined as any;
variableName = 'chunk';
private allDependencies = new Set<ExternalModule | Chunk>();

private dependencies = new Set<ExternalModule | Chunk>();
private dynamicDependencies = new Set<ExternalModule | Chunk>();
Expand Down Expand Up @@ -470,11 +473,19 @@ export default class Chunk {
varOrConst: options.preferConst ? 'const' : 'var'
};

// because bindings are resolved to their exact chunk, we need all chunk dependencies directly
// available to the dependency analysis
this.allDependencies = new Set<Chunk | ExternalModule>();
this.gatherAllDependencies(this);
this.hasSideEffects = this.orderedModules.some(module => module.hasSideEffects);
// for static and dynamic entry points, inline the execution list to avoid loading latency
if (
options.hoistTransitiveImports !== false &&
!this.graph.preserveModules &&
this.facadeModule !== null
) {
for (const dep of this.dependencies) {
if (dep instanceof Chunk) this.inlineChunkDependencies(dep);
}
}
const sortedDependencies = [...this.dependencies];
sortByExecutionOrder(sortedDependencies);
this.dependencies = new Set(sortedDependencies);

this.prepareDynamicImports();
this.setIdentifierRenderResolutions(options);
Expand Down Expand Up @@ -564,7 +575,7 @@ export default class Chunk {

// populate ids in the rendered declarations only here
// as chunk ids known only after prerender
for (const dependency of this.allDependencies) {
for (const dependency of this.dependencies) {
if (dependency instanceof ExternalModule && !dependency.renormalizeRenderPath) continue;
const renderedDependency = this.renderedDependencies!.get(dependency)!;
const depId = dependency instanceof ExternalModule ? renderedDependency.id : dependency.id!;
Expand Down Expand Up @@ -605,22 +616,11 @@ export default class Chunk {
});
}

// when using inlineDependencies, orphaned pure chunks can be excluded
const dependencies = new Set([]);
const inlineDependencies =
options.hoistTransitiveImports !== false &&
!this.graph.preserveModules &&
this.facadeModule !== null;
const pureExternalModules = Boolean(this.graph.treeshakingOptions?.pureExternalModules);
this.gatherUsedDependencies(this, pureExternalModules, inlineDependencies, dependencies);

const magicString = finalise(
this.renderedSource!,
{
accessedGlobals,
dependencies: sortByExecutionOrder([...dependencies]).map(
dep => this.renderedDependencies?.get(dep) as ModuleDeclarationDependency
),
dependencies: [...this.renderedDependencies!.values()],
exports: this.renderedExports!,
hasExports,
indentString: this.indentString,
Expand Down Expand Up @@ -780,49 +780,6 @@ export default class Chunk {
}
}

private gatherAllDependencies(chunk: Chunk) {
for (const dep of chunk.dependencies) {
if (this.allDependencies.has(dep)) continue;
this.allDependencies.add(dep);
if (dep instanceof Chunk) {
this.gatherAllDependencies(dep);
}
}
}

private gatherUsedDependencies(
chunk: Chunk,
pureExternalModules: boolean,
inlineDependencies: boolean,
dependencies: Set<Chunk | ExternalModule>,
seen = new Set<Chunk | ExternalModule>()
) {
const doInline = inlineDependencies || chunk === this;
for (const dep of chunk.dependencies) {
if (seen.has(dep)) return;
seen.add(dep);

const hasImportedBinding = this.hasImportedBindingTo(dep);

if (dep instanceof ExternalModule) {
if (
hasImportedBinding ||
(doInline && (!pureExternalModules || chunk.hasImportedBindingTo(dep)))
)
dependencies.add(dep);
continue;
}

if (
hasImportedBinding ||
(doInline && (dep.hasSideEffects || chunk.hasImportedBindingTo(dep)))
)
dependencies.add(dep);

this.gatherUsedDependencies(dep, pureExternalModules, inlineDependencies, dependencies, seen);
}
}

private getChunkDependencyDeclarations(
options: OutputOptions
): Map<Chunk | ExternalModule, ModuleDeclarationDependency> {
Expand Down Expand Up @@ -859,7 +816,7 @@ export default class Chunk {
const renderedImports = new Set<Variable>();
const dependencies = new Map<Chunk | ExternalModule, ModuleDeclarationDependency>();

for (const dep of this.allDependencies) {
for (const dep of this.dependencies) {
const imports: ImportSpecifier[] = [];
for (const variable of this.imports) {
if (
Expand Down Expand Up @@ -1003,9 +960,14 @@ export default class Chunk {
return relativePath.startsWith('../') ? relativePath : './' + relativePath;
}

private hasImportedBindingTo(dep: Chunk | ExternalModule) {
const renderedDeclaration = this.renderedDependencies?.get(dep)!;
return renderedDeclaration.imports?.length || renderedDeclaration.reexports?.length;
private inlineChunkDependencies(chunk: Chunk) {
for (const dep of chunk.dependencies) {
if (this.dependencies.has(dep)) continue;
this.dependencies.add(dep);
if (dep instanceof Chunk) {
this.inlineChunkDependencies(dep);
}
}
}

private prepareDynamicImports() {
Expand All @@ -1026,7 +988,7 @@ export default class Chunk {
}

private setExternalRenderPaths(options: OutputOptions, inputBase: string) {
for (const dependency of [...this.allDependencies, ...this.dynamicDependencies]) {
for (const dependency of [...this.dependencies, ...this.dynamicDependencies]) {
if (dependency instanceof ExternalModule) {
dependency.setRenderPath(options, inputBase);
}
Expand Down
15 changes: 8 additions & 7 deletions src/Module.ts
Expand Up @@ -208,7 +208,6 @@ export default class Module {
exports: { [name: string]: ExportDescription } = Object.create(null);
exportsAll: { [name: string]: string } = Object.create(null);
facadeChunk: Chunk | null = null;
hasSideEffects = false;
importDescriptions: { [name: string]: ImportDescription } = Object.create(null);
importers: string[] = [];
importMetas: MetaProperty[] = [];
Expand Down Expand Up @@ -369,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.hasSideEffects)
) {
if (dependency instanceof ExternalModule || dependency.hasEffects()) {
relevantDependencies.add(dependency);
} else {
for (const transitiveDependency of dependency.dependencies) {
Expand Down Expand Up @@ -533,10 +529,15 @@ export default class Module {
return null as any;
}

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

include(): void {
const context = createInclusionContext();
this.hasSideEffects = this.ast.hasEffects(createHasEffectsContext());
if (this.hasSideEffects || this.ast.shouldBeIncluded(context)) this.ast.include(context, false);
if (this.ast.shouldBeIncluded(context)) this.ast.include(context, false);
}

includeAllExports() {
Expand Down
3 changes: 1 addition & 2 deletions src/utils/executionOrder.ts
Expand Up @@ -9,9 +9,8 @@ interface OrderedExecutionUnit {
const compareExecIndex = <T extends OrderedExecutionUnit>(unitA: T, unitB: T) =>
unitA.execIndex > unitB.execIndex ? 1 : -1;

export function sortByExecutionOrder<T extends OrderedExecutionUnit>(units: T[]): T[] {
export function sortByExecutionOrder(units: OrderedExecutionUnit[]) {
units.sort(compareExecIndex);
return units;
}

export function analyseModuleExecution(entryModules: Module[]) {
Expand Down
@@ -1,8 +1,8 @@
define(['exports', './sub/index'], function (exports, index$1) { 'use strict';
define(['exports', './sub/index'], function (exports, index) { 'use strict';

const baz = { bar: index$1.default };
const baz = { bar: index.default };

exports.foo = index$1.foo;
exports.foo = index.foo;
exports.baz = baz;

Object.defineProperty(exports, '__esModule', { value: true });
Expand Down
@@ -1,9 +1,9 @@
define(['exports', './components/sub/index', './components/index'], function (exports, index$1, index) { 'use strict';
define(['exports', './components/sub/index', './components/index'], function (exports, index, index$1) { 'use strict';



exports.foo = index$1.foo;
exports.baz = index.baz;
exports.foo = index.foo;
exports.baz = index$1.baz;

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

Expand Down
Expand Up @@ -2,9 +2,9 @@

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

var index$1 = require('./sub/index.js');
var index = require('./sub/index.js');

const baz = { bar: index$1.default };
const baz = { bar: index.default };

exports.foo = index$1.foo;
exports.foo = index.foo;
exports.baz = baz;
Expand Up @@ -2,10 +2,10 @@

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

var index$1 = require('./components/sub/index.js');
var index = require('./components/index.js');
var index = require('./components/sub/index.js');
var index$1 = require('./components/index.js');



exports.foo = index$1.foo;
exports.baz = index.baz;
exports.foo = index.foo;
exports.baz = index$1.baz;
1 change: 0 additions & 1 deletion test/cli/samples/watch/bundle-error/main.js

This file was deleted.

@@ -0,0 +1,12 @@
module.exports = {
description: 'prunes pure unused external imports ([#1352])',
expectedWarnings: ['DEPRECATED_FEATURE'],
options: {
strictDeprecations: false,
external: ['external', 'other'],
treeshake: { pureExternalModules: ['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';



})));
@@ -0,0 +1,7 @@
import { unused } from 'external';
import { notused } from 'other';

function alsoUnused () {
unused();
notused();
}
@@ -0,0 +1,12 @@
module.exports = {
description: 'prunes pure unused external imports ([#1352])',
expectedWarnings: ['DEPRECATED_FEATURE', 'EMPTY_BUNDLE'],
options: {
strictDeprecations: false,
external: ['external', 'other'],
treeshake: { pureExternalModules: 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';



})));

0 comments on commit d0312ff

Please sign in to comment.