Prevent final resolution and facade creation for inlined dynamic impo…

…rts (#2677)

* failing test for #2676

* skip final resolution if dynamic import has already been rendered

* add expected test output

* only mark dynamic import as replaced if it was completely overwritten

* * only render final resolutions for cross-chunk dynamic imports
* do not create facades for inlined dynamic imports

* Get rid of AppVeyor Node 8 test to speed up CI

appveyor.yml

@@ -11,7 +11,6 @@ environment:
   matrix:
     # node.js
     - nodejs_version: 6
-    - nodejs_version: 8
     - nodejs_version: 10
 
 install:

src/Chunk.ts

@@ -135,7 +135,7 @@ export default class Chunk {
 	private renderedSource: MagicStringBundle | null = null;
 	private renderedSourceLength: number = undefined;
 
-	constructor(graph: Graph, orderedModules: Module[], inlineDynamicImports: boolean) {
+	constructor(graph: Graph, orderedModules: Module[]) {
 		this.graph = graph;
 		this.orderedModules = orderedModules;
 		this.execIndex = orderedModules.length > 0 ? orderedModules[0].execIndex : Infinity;
@@ -149,7 +149,10 @@ export default class Chunk {
 				this.isManualChunk = true;
 			}
 			module.chunk = this;
-			if (module.isEntryPoint || (module.isDynamicEntryPoint && !inlineDynamicImports)) {
+			if (
+				module.isEntryPoint ||
+				module.dynamicallyImportedBy.some(module => orderedModules.indexOf(module) === -1)
+			) {
 				this.entryModules.push(module);
 			}
 		}
@@ -378,8 +381,8 @@ export default class Chunk {
 			for (const { node, resolution } of module.dynamicImports) {
 				if (!resolution) continue;
 				if (resolution instanceof Module) {
-					const resolutionChunk = resolution.facadeChunk || resolution.chunk;
-					if (resolutionChunk && resolutionChunk !== this && resolutionChunk.id) {
+					if (resolution.isIncluded() && resolution.chunk !== this) {
+						const resolutionChunk = resolution.facadeChunk || resolution.chunk;
 						let relPath = normalize(relative(dirname(this.id), resolutionChunk.id));
 						if (!relPath.startsWith('../')) relPath = './' + relPath;
 						node.renderFinalResolution(code, `'${relPath}'`);

src/Graph.ts

@@ -413,7 +413,7 @@ export default class Graph {
 				let chunks: Chunk[] = [];
 				if (this.preserveModules) {
 					for (const module of orderedModules) {
-						const chunk = new Chunk(this, [module], inlineDynamicImports);
+						const chunk = new Chunk(this, [module]);
 						if (module.isEntryPoint || !chunk.isEmpty) {
 							chunk.entryModules = [module];
 						}
@@ -434,7 +434,7 @@ export default class Graph {
 					for (const entryHashSum in chunkModules) {
 						const chunkModulesOrdered = chunkModules[entryHashSum];
 						sortByExecutionOrder(chunkModulesOrdered);
-						const chunk = new Chunk(this, chunkModulesOrdered, inlineDynamicImports);
+						const chunk = new Chunk(this, chunkModulesOrdered);
 						chunks.push(chunk);
 					}
 				}
@@ -463,7 +463,7 @@ export default class Graph {
 					for (const chunk of chunks) {
 						for (const entryModule of chunk.entryModules) {
 							if (chunk.facadeModule !== entryModule) {
-								const entryPointFacade = new Chunk(this, [], inlineDynamicImports);
+								const entryPointFacade = new Chunk(this, []);
 								entryPointFacade.turnIntoFacade(entryModule);
 								facades.push(entryPointFacade);
 							}

src/Module.ts

@@ -160,6 +160,7 @@ export default class Module {
 	comments: CommentDescription[] = [];
 	customTransformCache: boolean;
 	dependencies: (Module | ExternalModule)[] = [];
+	dynamicallyImportedBy: Module[] = [];
 	dynamicDependencies: (Module | ExternalModule)[] = [];
 	dynamicImports: {
 		node: Import;
@@ -179,7 +180,6 @@ export default class Module {
 	importDescriptions: { [name: string]: ImportDescription } = Object.create(null);
 	importMetas: MetaProperty[] = [];
 	imports = new Set<Variable>();
-	isDynamicEntryPoint: boolean = false;
 	isEntryPoint: boolean = false;
 	isExecuted: boolean = false;
 	isExternal: false;
@@ -621,7 +621,7 @@ export default class Module {
 		const resolution = this.dynamicImports.find(dynamicImport => dynamicImport.node === node)
 			.resolution;
 		if (resolution instanceof Module) {
-			resolution.isDynamicEntryPoint = true;
+			resolution.dynamicallyImportedBy.push(this);
 			resolution.includeAllExports();
 		}
 	}

src/rollup/index.ts

@@ -215,7 +215,8 @@ export default function rollup(rawInputOptions: GenericConfigObject): Promise<Ro
 									facadeModuleId: facadeModule && facadeModule.id,
 									fileName: chunk.id,
 									imports: chunk.getImportIds(),
-									isDynamicEntry: facadeModule !== null && facadeModule.isDynamicEntryPoint,
+									isDynamicEntry:
+										facadeModule !== null && facadeModule.dynamicallyImportedBy.length > 0,
 									isEntry: facadeModule !== null && facadeModule.isEntryPoint,
 									map: undefined,
 									modules: chunk.renderedModules,

src/utils/chunkColouring.ts

@@ -33,7 +33,7 @@ export function assignChunkColouringHashes(
 		for (const { resolution } of module.dynamicImports) {
 			if (
 				resolution instanceof Module &&
-				resolution.isDynamicEntryPoint &&
+				resolution.dynamicallyImportedBy.length > 0 &&
 				!resolution.chunkAlias
 			) {
 				dynamicImports.push(resolution);

test/chunking-form/samples/dynamic-import-statically-imported-2/_config.js

@@ -0,0 +1,7 @@
+module.exports = {
+	description:
+		'handles dynamic imports of previously statically imported chunks that are also dynamically imported by other chunks',
+	options: {
+		input: ['main.js', 'main2.js']
+	}
+};

test/chunking-form/samples/dynamic-import-statically-imported-2/_expected/amd/generated-chunk.js

@@ -0,0 +1,20 @@
+define(['require', 'exports'], function (require, exports) { 'use strict';
+
+	function foo() {
+		return 'dep2';
+	}
+
+	Promise.resolve().then(function () { return dep1; }).then(({ bar }) => console.log(bar()));
+
+	function bar() {
+		return foo();
+	}
+
+	var dep1 = /*#__PURE__*/Object.freeze({
+		bar: bar
+	});
+
+	exports.foo = foo;
+	exports.bar = bar;
+
+});

test/chunking-form/samples/dynamic-import-statically-imported-2/_expected/amd/generated-chunk2.js

@@ -0,0 +1,7 @@
+define(['exports', './generated-chunk.js'], function (exports, dep1_js) { 'use strict';
+
+
+
+	exports.bar = dep1_js.bar;
+
+});

test/chunking-form/samples/dynamic-import-statically-imported-2/_expected/amd/main.js

@@ -0,0 +1,5 @@
+define(['./generated-chunk.js'], function (dep1_js) { 'use strict';
+
+	console.log(dep1_js.foo(), dep1_js.bar());
+
+});

test/chunking-form/samples/dynamic-import-statically-imported-2/_expected/amd/main2.js

@@ -0,0 +1,5 @@
+define(['require'], function (require) { 'use strict';
+
+	new Promise(function (resolve, reject) { require(['./generated-chunk2.js'], resolve, reject) }).then(({ bar }) => console.log(bar()));
+
+});

test/chunking-form/samples/dynamic-import-statically-imported-2/_expected/cjs/generated-chunk.js

@@ -0,0 +1,18 @@
+'use strict';
+
+function foo() {
+	return 'dep2';
+}
+
+Promise.resolve().then(function () { return dep1; }).then(({ bar }) => console.log(bar()));
+
+function bar() {
+	return foo();
+}
+
+var dep1 = /*#__PURE__*/Object.freeze({
+	bar: bar
+});
+
+exports.foo = foo;
+exports.bar = bar;

test/chunking-form/samples/dynamic-import-statically-imported-2/_expected/cjs/generated-chunk2.js

@@ -0,0 +1,7 @@
+'use strict';
+
+var dep1_js = require('./generated-chunk.js');
+
+
+
+exports.bar = dep1_js.bar;

test/chunking-form/samples/dynamic-import-statically-imported-2/_expected/cjs/main.js

@@ -0,0 +1,5 @@
+'use strict';
+
+var dep1_js = require('./generated-chunk.js');
+
+console.log(dep1_js.foo(), dep1_js.bar());

test/chunking-form/samples/dynamic-import-statically-imported-2/_expected/cjs/main2.js

@@ -0,0 +1,3 @@
+'use strict';
+
+Promise.resolve(require('./generated-chunk2.js')).then(({ bar }) => console.log(bar()));

test/chunking-form/samples/dynamic-import-statically-imported-2/_expected/es/generated-chunk.js

@@ -0,0 +1,15 @@
+function foo() {
+	return 'dep2';
+}
+
+Promise.resolve().then(function () { return dep1; }).then(({ bar }) => console.log(bar()));
+
+function bar() {
+	return foo();
+}
+
+var dep1 = /*#__PURE__*/Object.freeze({
+	bar: bar
+});
+
+export { foo as a, bar as b };

test/chunking-form/samples/dynamic-import-statically-imported-2/_expected/es/generated-chunk2.js

@@ -0,0 +1 @@
+export { b as bar } from './generated-chunk.js';

test/chunking-form/samples/dynamic-import-statically-imported-2/_expected/es/main.js

@@ -0,0 +1,3 @@
+import { a as foo, b as bar } from './generated-chunk.js';
+
+console.log(foo(), bar());

test/chunking-form/samples/dynamic-import-statically-imported-2/_expected/es/main2.js

@@ -0,0 +1 @@
+import('./generated-chunk2.js').then(({ bar }) => console.log(bar()));

test/chunking-form/samples/dynamic-import-statically-imported-2/_expected/system/generated-chunk.js

@@ -0,0 +1,27 @@
+System.register([], function (exports, module) {
+	'use strict';
+	return {
+		execute: function () {
+
+			exports({
+				a: foo,
+				b: bar
+			});
+
+			function foo() {
+				return 'dep2';
+			}
+
+			Promise.resolve().then(function () { return dep1; }).then(({ bar }) => console.log(bar()));
+
+			function bar() {
+				return foo();
+			}
+
+			var dep1 = /*#__PURE__*/Object.freeze({
+				bar: bar
+			});
+
+		}
+	};
+});

test/chunking-form/samples/dynamic-import-statically-imported-2/_expected/system/generated-chunk2.js

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

test/chunking-form/samples/dynamic-import-statically-imported-2/_expected/system/main.js

@@ -0,0 +1,15 @@
+System.register(['./generated-chunk.js'], function (exports, module) {
+	'use strict';
+	var foo, bar;
+	return {
+		setters: [function (module) {
+			foo = module.a;
+			bar = module.b;
+		}],
+		execute: function () {
+
+			console.log(foo(), bar());
+
+		}
+	};
+});

test/chunking-form/samples/dynamic-import-statically-imported-2/_expected/system/main2.js

@@ -0,0 +1,10 @@
+System.register([], function (exports, module) {
+	'use strict';
+	return {
+		execute: function () {
+
+			module.import('./generated-chunk2.js').then(({ bar }) => console.log(bar()));
+
+		}
+	};
+});

test/chunking-form/samples/dynamic-import-statically-imported-2/dep1.js

@@ -0,0 +1,5 @@
+import {foo} from './dep2.js';
+
+export function bar() {
+	return foo();
+}

test/chunking-form/samples/dynamic-import-statically-imported-2/dep2.js

@@ -0,0 +1,5 @@
+export function foo() {
+	return 'dep2';
+}
+
+import('./dep1.js').then(({ bar }) => console.log(bar()));

test/chunking-form/samples/dynamic-import-statically-imported-2/main.js

@@ -0,0 +1,4 @@
+import { bar } from './dep1.js';
+import { foo } from './dep2.js';
+
+console.log(foo(), bar());

test/chunking-form/samples/dynamic-import-statically-imported-2/main2.js

@@ -0,0 +1 @@
+import('./dep1.js').then(({ bar }) => console.log(bar()));

test/chunking-form/samples/dynamic-import-statically-imported/_config.js

@@ -0,0 +1,6 @@
+module.exports = {
+	description: 'handles dynamic imports of previously statically imported chunks',
+	options: {
+		input: ['main.js']
+	}
+};

test/chunking-form/samples/dynamic-import-statically-imported/_expected/amd/generated-chunk.js

@@ -0,0 +1,20 @@
+define(['require', 'exports'], function (require, exports) { 'use strict';
+
+	function foo() {
+		return 'dep2';
+	}
+
+	Promise.resolve().then(function () { return dep1; }).then(({ bar }) => console.log(bar()));
+
+	function bar() {
+		return foo();
+	}
+
+	var dep1 = /*#__PURE__*/Object.freeze({
+		bar: bar
+	});
+
+	exports.foo = foo;
+	exports.bar = bar;
+
+});

test/chunking-form/samples/dynamic-import-statically-imported/_expected/amd/main.js

@@ -0,0 +1,5 @@
+define(['./generated-chunk.js'], function (__chunk_1) { 'use strict';
+
+	console.log(__chunk_1.foo(), __chunk_1.bar());
+
+});

test/chunking-form/samples/dynamic-import-statically-imported/_expected/cjs/generated-chunk.js

@@ -0,0 +1,18 @@
+'use strict';
+
+function foo() {
+	return 'dep2';
+}
+
+Promise.resolve().then(function () { return dep1; }).then(({ bar }) => console.log(bar()));
+
+function bar() {
+	return foo();
+}
+
+var dep1 = /*#__PURE__*/Object.freeze({
+	bar: bar
+});
+
+exports.foo = foo;
+exports.bar = bar;

test/chunking-form/samples/dynamic-import-statically-imported/_expected/cjs/main.js

@@ -0,0 +1,5 @@
+'use strict';
+
+var __chunk_1 = require('./generated-chunk.js');
+
+console.log(__chunk_1.foo(), __chunk_1.bar());

test/chunking-form/samples/dynamic-import-statically-imported/_expected/es/generated-chunk.js

@@ -0,0 +1,15 @@
+function foo() {
+	return 'dep2';
+}
+
+Promise.resolve().then(function () { return dep1; }).then(({ bar }) => console.log(bar()));
+
+function bar() {
+	return foo();
+}
+
+var dep1 = /*#__PURE__*/Object.freeze({
+	bar: bar
+});
+
+export { foo as a, bar as b };

test/chunking-form/samples/dynamic-import-statically-imported/_expected/es/main.js

@@ -0,0 +1,3 @@
+import { a as foo, b as bar } from './generated-chunk.js';
+
+console.log(foo(), bar());