Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid name conflicts when inlining dynamic imports nested in functions #3256

Merged
merged 3 commits into from Nov 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/ast/nodes/ImportExpression.ts
Expand Up @@ -12,11 +12,11 @@ interface DynamicImportMechanism {
}

export default class Import extends NodeBase {
inlineNamespace?: NamespaceVariable;
source!: ExpressionNode;
type!: NodeType.tImportExpression;

private exportMode: 'none' | 'named' | 'default' | 'auto' = 'auto';
private inlineNamespace?: NamespaceVariable;

hasEffects(): boolean {
return true;
Expand All @@ -26,6 +26,7 @@ export default class Import extends NodeBase {
if (!this.included) {
this.included = true;
this.context.includeDynamicImport(this);
this.scope.addAccessedDynamicImport(this);
}
this.source.include(context, includeChildrenRecursively);
}
Expand Down
46 changes: 32 additions & 14 deletions src/ast/scopes/ChildScope.ts
@@ -1,9 +1,11 @@
import { getSafeName } from '../../utils/safeName';
import ImportExpression from '../nodes/ImportExpression';
import { ExpressionEntity } from '../nodes/shared/Expression';
import Variable from '../variables/Variable';
import Scope from './Scope';

export default class ChildScope extends Scope {
accessedDynamicImports?: Set<ImportExpression>;
accessedGlobalVariablesByFormat?: Map<string, Set<string>>;
accessedOutsideVariables = new Map<string, Variable>();
parent: Scope;
Expand All @@ -14,11 +16,18 @@ export default class ChildScope extends Scope {
parent.children.push(this);
}

addAccessedGlobalsByFormat(globalsByFormat: { [format: string]: string[] }) {
let accessedGlobalVariablesByFormat = this.accessedGlobalVariablesByFormat;
if (!accessedGlobalVariablesByFormat) {
accessedGlobalVariablesByFormat = this.accessedGlobalVariablesByFormat = new Map();
addAccessedDynamicImport(importExpression: ImportExpression) {
(this.accessedDynamicImports || (this.accessedDynamicImports = new Set())).add(
importExpression
);
if (this.parent instanceof ChildScope) {
this.parent.addAccessedDynamicImport(importExpression);
}
}

addAccessedGlobalsByFormat(globalsByFormat: { [format: string]: string[] }) {
const accessedGlobalVariablesByFormat =
this.accessedGlobalVariablesByFormat || (this.accessedGlobalVariablesByFormat = new Map());
for (const format of Object.keys(globalsByFormat)) {
let accessedGlobalVariables = accessedGlobalVariablesByFormat.get(format);
if (!accessedGlobalVariables) {
Expand All @@ -36,21 +45,14 @@ export default class ChildScope extends Scope {

addNamespaceMemberAccess(name: string, variable: Variable) {
this.accessedOutsideVariables.set(name, variable);
if (this.parent instanceof ChildScope) {
this.parent.addNamespaceMemberAccess(name, variable);
}
(this.parent as ChildScope).addNamespaceMemberAccess(name, variable);
}

addReturnExpression(expression: ExpressionEntity) {
this.parent instanceof ChildScope && this.parent.addReturnExpression(expression);
}

contains(name: string): boolean {
return this.variables.has(name) || this.parent.contains(name);
}

deconflict(format: string) {
const usedNames = new Set<string>();
addUsedOutsideNames(usedNames: Set<string>, format: string): void {
for (const variable of this.accessedOutsideVariables.values()) {
if (variable.included) {
usedNames.add(variable.getBaseVariableName());
Expand All @@ -66,6 +68,22 @@ export default class ChildScope extends Scope {
usedNames.add(name);
}
}
}

contains(name: string): boolean {
return this.variables.has(name) || this.parent.contains(name);
}

deconflict(format: string) {
const usedNames = new Set<string>();
this.addUsedOutsideNames(usedNames, format);
if (this.accessedDynamicImports) {
for (const importExpression of this.accessedDynamicImports) {
if (importExpression.inlineNamespace) {
usedNames.add(importExpression.inlineNamespace.getBaseVariableName());
}
}
}
for (const [name, variable] of this.variables) {
if (variable.included || variable.alwaysRendered) {
variable.setSafeName(getSafeName(name, usedNames));
Expand All @@ -77,7 +95,7 @@ export default class ChildScope extends Scope {
}

findLexicalBoundary(): ChildScope {
return this.parent instanceof ChildScope ? this.parent.findLexicalBoundary() : this;
return (this.parent as ChildScope).findLexicalBoundary();
}

findVariable(name: string): Variable {
Expand Down
30 changes: 7 additions & 23 deletions src/utils/deconflictChunk.ts
Expand Up @@ -31,7 +31,9 @@ export function deconflictChunk(
interop: boolean,
preserveModules: boolean
) {
addUsedGlobalNames(usedNames, modules, format);
for (const module of modules) {
module.scope.addUsedOutsideNames(usedNames, format);
}
deconflictTopLevelVariables(usedNames, modules);
DECONFLICT_IMPORTED_VARIABLES_BY_FORMAT[format](
usedNames,
Expand All @@ -46,25 +48,6 @@ export function deconflictChunk(
}
}

function addUsedGlobalNames(usedNames: Set<string>, modules: Module[], format: string) {
for (const module of modules) {
const moduleScope = module.scope;
for (const [name, variable] of moduleScope.accessedOutsideVariables) {
if (variable.included) {
usedNames.add(name);
}
}
const accessedGlobalVariables =
moduleScope.accessedGlobalVariablesByFormat &&
moduleScope.accessedGlobalVariablesByFormat.get(format);
if (accessedGlobalVariables) {
for (const name of accessedGlobalVariables) {
usedNames.add(name);
}
}
}
}

function deconflictImportsEsm(
usedNames: Set<string>,
imports: Set<Variable>,
Expand Down Expand Up @@ -114,9 +97,10 @@ function deconflictImportsOther(
if (chunk.exportMode === 'default' || (preserveModules && variable.isNamespace)) {
variable.setRenderNames(null, chunk.variableName);
} else {
variable.setRenderNames(chunk.variableName, chunk.getVariableExportName(variable) as
| string
| null);
variable.setRenderNames(
chunk.variableName,
chunk.getVariableExportName(variable) as string | null
);
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions test/form/samples/nested-inlined-dynamic-import/_config.js
@@ -0,0 +1,6 @@
module.exports = {
description: 'deconflicts variables when nested dynamic imports are inlined',
options: {
inlineDynamicImports: true
}
};
18 changes: 18 additions & 0 deletions test/form/samples/nested-inlined-dynamic-import/_expected/amd.js
@@ -0,0 +1,18 @@
define(function () { 'use strict';

async function main() {
const foo$1 = 1;
const ns = await Promise.resolve().then(function () { return foo; });
console.log(ns.value + foo$1);
}

main();

const value = 42;

var foo = /*#__PURE__*/Object.freeze({
__proto__: null,
value: value
});

});
16 changes: 16 additions & 0 deletions test/form/samples/nested-inlined-dynamic-import/_expected/cjs.js
@@ -0,0 +1,16 @@
'use strict';

async function main() {
const foo$1 = 1;
const ns = await Promise.resolve().then(function () { return foo; });
console.log(ns.value + foo$1);
}

main();

const value = 42;

var foo = /*#__PURE__*/Object.freeze({
__proto__: null,
value: value
});
14 changes: 14 additions & 0 deletions test/form/samples/nested-inlined-dynamic-import/_expected/es.js
@@ -0,0 +1,14 @@
async function main() {
const foo$1 = 1;
const ns = await Promise.resolve().then(function () { return foo; });
console.log(ns.value + foo$1);
}

main();

const value = 42;

var foo = /*#__PURE__*/Object.freeze({
__proto__: null,
value: value
});
19 changes: 19 additions & 0 deletions test/form/samples/nested-inlined-dynamic-import/_expected/iife.js
@@ -0,0 +1,19 @@
(function () {
'use strict';

async function main() {
const foo$1 = 1;
const ns = await Promise.resolve().then(function () { return foo; });
console.log(ns.value + foo$1);
}

main();

const value = 42;

var foo = /*#__PURE__*/Object.freeze({
__proto__: null,
value: value
});

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

async function main() {
const foo$1 = 1;
const ns = await Promise.resolve().then(function () { return foo; });
console.log(ns.value + foo$1);
}

main();

const value = 42;

var foo = /*#__PURE__*/Object.freeze({
__proto__: null,
value: value
});

}
};
});
21 changes: 21 additions & 0 deletions test/form/samples/nested-inlined-dynamic-import/_expected/umd.js
@@ -0,0 +1,21 @@
(function (factory) {
typeof define === 'function' && define.amd ? define(factory) :
factory();
}((function () { 'use strict';

async function main() {
const foo$1 = 1;
const ns = await Promise.resolve().then(function () { return foo; });
console.log(ns.value + foo$1);
}

main();

const value = 42;

var foo = /*#__PURE__*/Object.freeze({
__proto__: null,
value: value
});

})));
1 change: 1 addition & 0 deletions test/form/samples/nested-inlined-dynamic-import/foo.js
@@ -0,0 +1 @@
export const value = 42;
7 changes: 7 additions & 0 deletions test/form/samples/nested-inlined-dynamic-import/main.js
@@ -0,0 +1,7 @@
async function main() {
const foo = 1;
const ns = await import('./foo.js');
console.log(ns.value + foo);
}

main();
12 changes: 12 additions & 0 deletions test/function/samples/nested-inlined-dynamic-import-1/_config.js
@@ -0,0 +1,12 @@
const assert = require('assert');

module.exports = {
description:
'deconflicts variables when nested dynamic imports are inlined via inlineDynamicImports',
options: {
inlineDynamicImports: true
},
exports(exports) {
return exports().then(result => assert.strictEqual(result, 43));
}
};
@@ -0,0 +1 @@
export const value = 42;
4 changes: 4 additions & 0 deletions test/function/samples/nested-inlined-dynamic-import-1/main.js
@@ -0,0 +1,4 @@
export default () => {
const foo = 1;
return import('./foo.js').then(ns => ns.value + foo);
};
16 changes: 16 additions & 0 deletions test/function/samples/nested-inlined-dynamic-import-2/_config.js
@@ -0,0 +1,16 @@
const assert = require('assert');

module.exports = {
description: 'deconflicts variables when nested dynamic imports are inlined',
warnings: [
{
code: 'CIRCULAR_DEPENDENCY',
cycle: ['lib1.js', 'lib2.js', 'lib1.js'],
importer: 'lib1.js',
message: 'Circular dependency: lib1.js -> lib2.js -> lib1.js'
}
],
exports(exports) {
return exports().then(result => assert.strictEqual(result, 43));
}
};
6 changes: 6 additions & 0 deletions test/function/samples/nested-inlined-dynamic-import-2/lib1.js
@@ -0,0 +1,6 @@
import './lib2.js';

export default () => {
const lib2 = 1;
return import('./lib2.js').then(ns => ns.foo + lib2);
};
3 changes: 3 additions & 0 deletions test/function/samples/nested-inlined-dynamic-import-2/lib2.js
@@ -0,0 +1,3 @@
import './lib1.js';

export const foo = 42;
@@ -0,0 +1 @@
export {default} from './lib1.js';