Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Inline dynamic imports that are also imported statically and only use…
…d in a single chunk (#2295)
  • Loading branch information
guybedford authored and lukastaegert committed Jun 27, 2018
1 parent e13b552 commit c3228cd
Show file tree
Hide file tree
Showing 11 changed files with 88 additions and 7 deletions.
10 changes: 6 additions & 4 deletions src/Chunk.ts
Expand Up @@ -214,6 +214,11 @@ export default class Chunk {
const declaration = module.imports[importName];
this.traceImport(declaration.name, declaration.module);
}
for (const { resolution } of module.dynamicImportResolutions) {
this.hasDynamicImport = true;
if (resolution instanceof Module && resolution.chunk === this)
resolution.getOrCreateNamespace().include();
}
}

// Note preserveModules implementation is not a comprehensive technique
Expand Down Expand Up @@ -397,7 +402,6 @@ export default class Chunk {
for (let i = 0; i < module.dynamicImportResolutions.length; i++) {
const node = module.dynamicImports[i];
const resolution = module.dynamicImportResolutions[i].resolution;
this.hasDynamicImport = true;

if (!resolution) continue;

Expand All @@ -406,7 +410,6 @@ export default class Chunk {
// ensuring that we create a namespace import of it as well
if (resolution.chunk === this) {
const namespace = resolution.getOrCreateNamespace();
namespace.include();
node.setResolution(false, namespace.getName());
// for the module in another chunk, import that other chunk directly
} else {
Expand Down Expand Up @@ -772,8 +775,6 @@ export default class Chunk {
const n = options.compact ? '' : '\n';
const _ = options.compact ? '' : ' ';

this.prepareDynamicImports();

const renderOptions: RenderOptions = {
compact: options.compact,
freeze: options.freeze !== false,
Expand Down Expand Up @@ -807,6 +808,7 @@ export default class Chunk {
}

this.setIdentifierRenderResolutions(options);
this.prepareDynamicImports();

let hoistedSource = '';

Expand Down
17 changes: 15 additions & 2 deletions src/Graph.ts
Expand Up @@ -28,7 +28,12 @@ import {
import { Asset, createAssetPluginHooks, finaliseAsset } from './utils/assetHooks';
import { load, makeOnwarn, resolveId } from './utils/defaults';
import ensureArray from './utils/ensureArray';
import { randomUint8Array, Uint8ArrayToHexString, Uint8ArrayXor } from './utils/entryHashing';
import {
randomUint8Array,
Uint8ArrayEqual,
Uint8ArrayToHexString,
Uint8ArrayXor
} from './utils/entryHashing';
import error from './utils/error';
import first from './utils/first';
import { isRelative, relative, resolve } from './utils/path';
Expand Down Expand Up @@ -537,7 +542,15 @@ export default class Graph {
}

for (const dynamicModule of module.dynamicImportResolutions) {
if (dynamicModule.resolution instanceof Module) {
if (!(dynamicModule.resolution instanceof Module)) continue;
// If the parent module of a dynamic import is to a child module whose graph
// colouring is the same as the parent module, then that dynamic import does
// not need to be treated as a new entry point as it is in the static graph
if (
!graphColouring ||
(!dynamicModule.resolution.chunkAlias &&
!Uint8ArrayEqual(dynamicModule.resolution.entryPointsHash, curEntry.entryPointsHash))
) {
if (dynamicImports.indexOf(dynamicModule.resolution) === -1) {
dynamicImports.push(dynamicModule.resolution);
dynamicImportAliases.push(dynamicModule.alias);
Expand Down
7 changes: 7 additions & 0 deletions src/utils/entryHashing.ts
Expand Up @@ -29,6 +29,13 @@ export function randomUint8Array(len: number) {
return buffer;
}

export function Uint8ArrayEqual(bufferA: Uint8Array, bufferB: Uint8Array) {
for (let i = 0; i < bufferA.length; i++) {
if (bufferA[i] !== bufferB[i]) return false;
}
return true;
}

export function randomHexString(len: number) {
return Uint8ArrayToHexString(randomUint8Array(Math.floor(len / 2)));
}
@@ -0,0 +1,6 @@
module.exports = {
description: 'Dynamic import inlining for static colouring',
options: {
input: ['main.js']
}
};
@@ -0,0 +1,13 @@
define(['require'], function (require) { 'use strict';

var foo = "FOO";

var foo$1 = /*#__PURE__*/Object.freeze({
default: foo
});

var main = Promise.resolve().then(function () { return foo$1; });

return main;

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

var foo = "FOO";

var foo$1 = /*#__PURE__*/Object.freeze({
default: foo
});

var main = Promise.resolve().then(function () { return foo$1; });

module.exports = main;
@@ -0,0 +1,9 @@
var foo = "FOO";

var foo$1 = /*#__PURE__*/Object.freeze({
default: foo
});

var main = Promise.resolve().then(function () { return foo$1; });

export default main;
@@ -0,0 +1,16 @@
System.register([], function (exports, module) {
'use strict';
return {
execute: function () {

var foo = "FOO";

var foo$1 = /*#__PURE__*/Object.freeze({
default: foo
});

var main = exports('default', Promise.resolve().then(function () { return foo$1; }));

}
};
});
@@ -0,0 +1 @@
export default "FOO";
@@ -0,0 +1,3 @@
import foo from "./foo.js";

export default import("./foo.js");
2 changes: 1 addition & 1 deletion test/form/samples/dynamic-import-inlining/_expected.js
Expand Up @@ -5,6 +5,6 @@ var foo$1 = /*#__PURE__*/Object.freeze({
});

const bar = 2;
Promise.resolve().then(function () { return foo; });
Promise.resolve().then(function () { return foo$1; });

export { bar };

0 comments on commit c3228cd

Please sign in to comment.