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

Reimplement variable deconflicting logic #2689

Merged
merged 16 commits into from Feb 17, 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
2 changes: 1 addition & 1 deletion rollup.config.js
Expand Up @@ -40,7 +40,7 @@ const onwarn = warning => {
throw new Error(warning.message);
};

const expectedAcornImport = "import acorn__default, { tokTypes, Parser } from 'acorn';";
const expectedAcornImport = /import acorn__default, { tokTypes, Parser( as [\w$]+)? } from 'acorn';/;
const newAcornImport =
"import * as acorn__default from 'acorn';\nimport { tokTypes, Parser } from 'acorn';";

Expand Down
148 changes: 31 additions & 117 deletions src/Chunk.ts
Expand Up @@ -22,6 +22,7 @@ import {
import { Addons } from './utils/addons';
import { toBase64 } from './utils/base64';
import collapseSourcemaps from './utils/collapseSourcemaps';
import { deconflictChunk } from './utils/deconflictChunk';
import { error } from './utils/error';
import { sortByExecutionOrder } from './utils/executionOrder';
import getIndentString from './utils/getIndentString';
Expand All @@ -30,7 +31,7 @@ import { basename, dirname, isAbsolute, normalize, relative, resolve } from './u
import renderChunk from './utils/renderChunk';
import { RenderOptions } from './utils/renderHelpers';
import { makeUnique, renderNamePattern } from './utils/renderNamePattern';
import { sanitizeFileName } from './utils/sanitize-file-name';
import { sanitizeFileName } from './utils/sanitizeFileName';
import { timeEnd, timeStart } from './utils/timers';
import { MISSING_EXPORT_SHIM_VARIABLE } from './utils/variableNames';

Expand Down Expand Up @@ -410,126 +411,41 @@ export default class Chunk {
}

private setIdentifierRenderResolutions(options: OutputOptions) {
this.needsExportsShim = false;
const used = Object.create(null);
const esm = options.format === 'es' || options.format === 'system';

// ensure no conflicts with globals
Object.keys(this.graph.scope.variables).forEach(name => (used[name] = 1));

function getSafeName(name: string): string {
let safeName = name;
while (used[safeName]) {
safeName = `${name}$${toBase64(used[name]++, true)}`;
}
used[safeName] = 1;
return safeName;
}

// reserved internal binding names for system format wiring
if (options.format === 'system') {
used._setter = used._starExcludes = used._$p = 1;
}

const toDeshadow: Set<string> = new Set();

if (!esm) {
this.dependencies.forEach(module => {
const safeName = getSafeName(module.variableName);
toDeshadow.add(safeName);
module.variableName = safeName;
});
}

for (const exportName of Object.keys(this.exportNames)) {
const exportVariable = this.exportNames[exportName];
if (exportVariable instanceof ExportShimVariable) this.needsExportsShim = true;
if (exportVariable && exportVariable.exportName !== exportName) {
exportVariable.exportName = exportName;
}
}

for (const variable of Array.from(this.imports)) {
let safeName;

if (variable.module instanceof ExternalModule) {
if (variable.name === '*') {
safeName = variable.module.variableName;
} else if (variable.name === 'default') {
if (
options.interop !== false &&
(variable.module.exportsNamespace || (!esm && variable.module.exportsNames))
) {
safeName = `${variable.module.variableName}__default`;
} else {
safeName = variable.module.variableName;
}
} else {
safeName = esm ? variable.name : `${variable.module.variableName}.${variable.name}`;
if (exportVariable) {
if (exportVariable instanceof ExportShimVariable) {
this.needsExportsShim = true;
}
if (esm) {
safeName = getSafeName(safeName);
toDeshadow.add(safeName);
}
} else if (esm) {
safeName = getSafeName(variable.name);
toDeshadow.add(safeName);
} else {
const chunk = variable.module.chunk;
exportVariable.exportName = exportName;
if (
chunk.exportMode === 'default' ||
(this.graph.preserveModules && variable.isNamespace)
options.format !== 'es' &&
options.format !== 'system' &&
exportVariable.isReassigned &&
!exportVariable.isId &&
(!isExportDefaultVariable(exportVariable) || !exportVariable.hasId)
) {
safeName = chunk.variableName;
exportVariable.setRenderNames('exports', exportName);
} else {
safeName = `${chunk.variableName}.${variable.module.chunk.getVariableExportName(
variable
)}`;
exportVariable.setRenderNames(null, null);
}
}
if (safeName) variable.setSafeName(safeName);
}

this.orderedModules.forEach(module => {
this.deconflictExportsOfModule(module, getSafeName, esm);
});

this.graph.scope.deshadow(toDeshadow, this.orderedModules.map(module => module.scope));
}

private deconflictExportsOfModule(
module: Module,
getSafeName: (name: string) => string,
esm: boolean
) {
Object.keys(module.scope.variables).forEach(variableName => {
const variable = module.scope.variables[variableName];
if (isExportDefaultVariable(variable) && variable.referencesOriginal()) {
variable.setSafeName(null);
return;
}
if (!(isExportDefaultVariable(variable) && variable.hasId)) {
let safeName;
if (esm || !variable.isReassigned || variable.isId) {
safeName = getSafeName(variable.name);
} else {
const safeExportName = variable.exportName;
if (safeExportName) {
safeName = `exports.${safeExportName}`;
} else {
safeName = getSafeName(variable.name);
}
}
variable.setSafeName(safeName);
}
});
module.exportShimVariable.setSafeName(null);

// deconflict reified namespaces
const namespace = module.getOrCreateNamespace();
if (namespace.included) {
namespace.setSafeName(getSafeName(namespace.name));
const usedNames = Object.create(null);
if (this.needsExportsShim) {
usedNames[MISSING_EXPORT_SHIM_VARIABLE] = true;
}

deconflictChunk(
this.orderedModules,
this.dependencies,
this.imports,
usedNames,
options.format,
options.interop !== false,
this.graph.preserveModules
);
}

private getChunkDependencyDeclarations(
Expand Down Expand Up @@ -573,13 +489,11 @@ export default class Chunk {
if (importsFromDependency.length) {
imports = [];
for (const variable of importsFromDependency) {
const local = variable.safeName || variable.name;
let imported;
if (variable.module instanceof ExternalModule) {
imported = variable.name;
} else {
imported = variable.module.chunk.getVariableExportName(variable);
}
const local = variable.getName();
const imported =
variable.module instanceof ExternalModule
? variable.name
: variable.module.chunk.getVariableExportName(variable);
imports.push({ local, imported });
}
}
Expand Down
31 changes: 25 additions & 6 deletions src/Graph.ts
Expand Up @@ -31,7 +31,6 @@ import { createPluginDriver, PluginDriver } from './utils/pluginDriver';
import relativeId, { getAliasName } from './utils/relativeId';
import { timeEnd, timeStart } from './utils/timers';
import transform from './utils/transform';
import { MISSING_EXPORT_SHIM_VARIABLE } from './utils/variableNames';

function makeOnwarn() {
const warned = Object.create(null);
Expand Down Expand Up @@ -157,12 +156,7 @@ export default class Graph {
}

this.shimMissingExports = options.shimMissingExports;

this.scope = new GlobalScope();
for (const name of ['module', 'exports', '_interopDefault', MISSING_EXPORT_SHIM_VARIABLE]) {
this.scope.findVariable(name); // creates global variable as side-effect
}

this.context = String(options.context);

const optionsModuleContext = options.moduleContext;
Expand Down Expand Up @@ -255,6 +249,31 @@ export default class Graph {
for (const module of this.modules) {
module.bindReferences();
}
this.warnForMissingExports();
}

private warnForMissingExports() {
for (const module of this.modules) {
for (const importName of Object.keys(module.importDescriptions)) {
const importDescription = module.importDescriptions[importName];
if (
importDescription.name !== '*' &&
!importDescription.module.getVariableForExportName(importDescription.name)
) {
module.warn(
{
code: 'NON_EXISTENT_EXPORT',
name: importDescription.name,
source: importDescription.module.id,
message: `Non-existent export '${
importDescription.name
}' is imported from ${relativeId(importDescription.module.id)}`
},
importDescription.start
);
}
}
}
}

includeMarked(modules: Module[]) {
Expand Down
2 changes: 1 addition & 1 deletion src/Module.ts
Expand Up @@ -751,7 +751,7 @@ export default class Module {
}
}

private warn(warning: RollupWarning, pos: number) {
warn(warning: RollupWarning, pos: number) {
if (pos !== undefined) {
warning.pos = pos;

Expand Down
3 changes: 2 additions & 1 deletion src/ast/nodes/BlockStatement.ts
Expand Up @@ -2,6 +2,7 @@ import MagicString from 'magic-string';
import { RenderOptions, renderStatementList } from '../../utils/renderHelpers';
import { ExecutionPathOptions } from '../ExecutionPathOptions';
import BlockScope from '../scopes/BlockScope';
import ChildScope from '../scopes/ChildScope';
import Scope from '../scopes/Scope';
import { UNKNOWN_EXPRESSION } from '../values';
import * as NodeType from './NodeType';
Expand All @@ -24,7 +25,7 @@ export default class BlockStatement extends StatementBase {

createScope(parentScope: Scope) {
this.scope = (<Node>this.parent).preventChildBlockScope
? parentScope
? <ChildScope>parentScope
: new BlockScope(parentScope);
}

Expand Down
9 changes: 5 additions & 4 deletions src/ast/nodes/ClassDeclaration.ts
@@ -1,5 +1,6 @@
import MagicString from 'magic-string';
import { RenderOptions } from '../../utils/renderHelpers';
import ChildScope from '../scopes/ChildScope';
import Identifier from './Identifier';
import * as NodeType from './NodeType';
import ClassNode from './shared/ClassNode';
Expand All @@ -22,10 +23,10 @@ export default class ClassDeclaration extends ClassNode {

parseNode(esTreeNode: GenericEsTreeNode) {
if (esTreeNode.id !== null) {
this.id = <Identifier>new this.context.nodeConstructors.Identifier(
esTreeNode.id,
this,
this.scope.parent
this.id = <Identifier>(
new this.context.nodeConstructors.Identifier(esTreeNode.id, this, <ChildScope>(
this.scope.parent
))
);
}
super.parseNode(esTreeNode);
Expand Down
2 changes: 2 additions & 0 deletions src/ast/nodes/ExportDefaultDeclaration.ts
Expand Up @@ -5,6 +5,7 @@ import {
NodeRenderOptions,
RenderOptions
} from '../../utils/renderHelpers';
import ModuleScope from '../scopes/ModuleScope';
import ExportDefaultVariable from '../variables/ExportDefaultVariable';
import ClassDeclaration, { isClassDeclaration } from './ClassDeclaration';
import FunctionDeclaration, { isFunctionDeclaration } from './FunctionDeclaration';
Expand Down Expand Up @@ -39,6 +40,7 @@ export function isExportDefaultDeclaration(node: Node): node is ExportDefaultDec
export default class ExportDefaultDeclaration extends NodeBase {
type: NodeType.tExportDefaultDeclaration;
declaration: FunctionDeclaration | ClassDeclaration | ExpressionNode;
scope: ModuleScope;

needsBoundaries: true;
variable: ExportDefaultVariable;
Expand Down
9 changes: 5 additions & 4 deletions src/ast/nodes/FunctionDeclaration.ts
@@ -1,3 +1,4 @@
import ChildScope from '../scopes/ChildScope';
import Identifier from './Identifier';
import * as NodeType from './NodeType';
import FunctionNode from './shared/FunctionNode';
Expand All @@ -19,10 +20,10 @@ export default class FunctionDeclaration extends FunctionNode {

parseNode(esTreeNode: GenericEsTreeNode) {
if (esTreeNode.id !== null) {
this.id = <Identifier>new this.context.nodeConstructors.Identifier(
esTreeNode.id,
this,
this.scope.parent
this.id = <Identifier>(
new this.context.nodeConstructors.Identifier(esTreeNode.id, this, <ChildScope>(
this.scope.parent
))
);
}
super.parseNode(esTreeNode);
Expand Down
9 changes: 9 additions & 0 deletions src/ast/nodes/MemberExpression.ts
Expand Up @@ -60,6 +60,14 @@ function getPathIfNotComputed(memberExpression: MemberExpression): PathWithPosit
return null;
}

function getStringFromPath(path: PathWithPositions): string {
let pathString = path[0].key;
for (let index = 1; index < path.length; index++) {
pathString += '.' + path[index].key;
}
return pathString;
}

export function isMemberExpression(node: Node): node is MemberExpression {
return node.type === NodeType.MemberExpression;
}
Expand Down Expand Up @@ -92,6 +100,7 @@ export default class MemberExpression extends NodeBase implements DeoptimizableE
(<ExternalVariable>resolvedVariable).module.suggestName(path[0].key);
}
this.variable = resolvedVariable;
this.scope.addNamespaceMemberAccess(getStringFromPath(path), resolvedVariable);
}
} else {
super.bind();
Expand Down
3 changes: 2 additions & 1 deletion src/ast/nodes/ObjectExpression.ts
@@ -1,6 +1,7 @@
import MagicString from 'magic-string';
import { BLANK } from '../../utils/blank';
import { NodeRenderOptions, RenderOptions } from '../../utils/renderHelpers';
import { NameCollection } from '../../utils/reservedNames';
import CallOptions from '../CallOptions';
import { DeoptimizableEntity } from '../DeoptimizableEntity';
import { ExecutionPathOptions } from '../ExecutionPathOptions';
Expand Down Expand Up @@ -45,7 +46,7 @@ export default class ObjectExpression extends NodeBase {
private propertyMap: PropertyMap | null;
private unmatchablePropertiesRead: (Property | SpreadElement)[] | null;
private unmatchablePropertiesWrite: Property[] | null;
private deoptimizedPaths: { [key: string]: true };
private deoptimizedPaths: NameCollection;
private hasUnknownDeoptimizedProperty: boolean;
private expressionsToBeDeoptimized: { [key: string]: DeoptimizableEntity[] };

Expand Down
4 changes: 3 additions & 1 deletion src/ast/nodes/ThisExpression.ts
@@ -1,6 +1,7 @@
import MagicString from 'magic-string';
import { RenderOptions } from '../../utils/renderHelpers';
import { ExecutionPathOptions } from '../ExecutionPathOptions';
import ModuleScope from '../scopes/ModuleScope';
import { ObjectPath } from '../values';
import ThisVariable from '../variables/ThisVariable';
import * as NodeType from './NodeType';
Expand Down Expand Up @@ -28,7 +29,8 @@ export default class ThisExpression extends NodeBase {
initialise() {
this.included = false;
this.variable = null;
this.alias = this.scope.findLexicalBoundary().isModuleScope ? this.context.moduleContext : null;
this.alias =
this.scope.findLexicalBoundary() instanceof ModuleScope ? this.context.moduleContext : null;
if (this.alias === 'undefined') {
this.context.warn(
{
Expand Down