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

Feat: add syntheticNamedExports #3295

Merged
merged 18 commits into from Jan 4, 2020
Merged
60 changes: 54 additions & 6 deletions src/Module.ts
Expand Up @@ -22,9 +22,11 @@ import TemplateLiteral from './ast/nodes/TemplateLiteral';
import VariableDeclaration from './ast/nodes/VariableDeclaration';
import ModuleScope from './ast/scopes/ModuleScope';
import { PathTracker, UNKNOWN_PATH } from './ast/utils/PathTracker';
import ExportDefaultVariable from './ast/variables/ExportDefaultVariable';
import ExportShimVariable from './ast/variables/ExportShimVariable';
import ExternalVariable from './ast/variables/ExternalVariable';
import NamespaceVariable from './ast/variables/NamespaceVariable';
import SyntheticNamedExport from './ast/variables/SyntheticNamedExport';
import Variable from './ast/variables/Variable';
import Chunk from './Chunk';
import ExternalModule from './ExternalModule';
Expand All @@ -39,7 +41,7 @@ import {
RollupWarning,
TransformModuleJSON
} from './rollup/types';
import { error } from './utils/error';
import { error, Errors } from './utils/error';
import getCodeFrame from './utils/getCodeFrame';
import { getOriginalLocation } from './utils/getOriginalLocation';
import { makeLegal } from './utils/identifierHelpers';
Expand Down Expand Up @@ -206,6 +208,7 @@ export default class Module {
scope!: ModuleScope;
sourcemapChain!: DecodedSourceMapOrMissing[];
sources = new Set<string>();
syntheticNamedExports: boolean;
transformFiles?: EmittedFile[];
userChunkNames = new Set<string>();
usesTopLevelAwait = false;
Expand All @@ -214,19 +217,28 @@ export default class Module {
private ast!: Program;
private astContext!: AstContext;
private context: string;
private defaultExport: ExportDefaultVariable | null | undefined = null;
private esTreeAst!: ESTree.Program;
private graph: Graph;
private magicString!: MagicString;
private namespaceVariable: NamespaceVariable | null = null;
private syntheticExports = new Map<string, SyntheticNamedExport>();
private transformDependencies: string[] = [];
private transitiveReexports: string[] | null = null;

constructor(graph: Graph, id: string, moduleSideEffects: boolean, isEntry: boolean) {
constructor(
graph: Graph,
id: string,
moduleSideEffects: boolean,
syntheticNamedExports: boolean,
isEntry: boolean
) {
this.id = id;
this.graph = graph;
this.excludeFromSourcemap = /\0/.test(id);
this.context = graph.getModuleContext(id);
this.moduleSideEffects = moduleSideEffects;
this.syntheticNamedExports = syntheticNamedExports;
this.isEntryPoint = isEntry;
}

Expand Down Expand Up @@ -299,6 +311,21 @@ export default class Module {
return allExportNames;
}

getDefaultExport() {
if (this.defaultExport === null) {
this.defaultExport = undefined;
this.defaultExport = this.astContext.traceExport('default') as ExportDefaultVariable;
manucorporat marked this conversation as resolved.
Show resolved Hide resolved
if (!this.defaultExport) {
error({
manucorporat marked this conversation as resolved.
Show resolved Hide resolved
code: Errors.SYNTHETIC_NAMED_EXPORTS_NEED_DEFAULT,
id: this.id,
message: `Modules with 'syntheticNamedExports' need a default export.`
});
}
}
return this.defaultExport;
}

getDynamicImportExpressions(): (string | Node)[] {
return this.dynamicImports.map(({ node }) => {
const importArgument = node.source;
Expand Down Expand Up @@ -342,7 +369,7 @@ export default class Module {

getOrCreateNamespace(): NamespaceVariable {
if (!this.namespaceVariable) {
this.namespaceVariable = new NamespaceVariable(this.astContext);
this.namespaceVariable = new NamespaceVariable(this.astContext, this.syntheticNamedExports);
this.namespaceVariable.initialise();
}
return this.namespaceVariable;
Expand Down Expand Up @@ -439,9 +466,25 @@ export default class Module {

// we don't want to create shims when we are just
// probing export * modules for exports
if (this.graph.shimMissingExports && !isExportAllSearch) {
this.shimMissingExport(name);
return this.exportShimVariable;
if (!isExportAllSearch) {
if (this.syntheticNamedExports) {
let syntheticExport = this.syntheticExports.get(name);
if (!syntheticExport && !this.exports[name]) {
const defaultExport = this.getDefaultExport();
if (defaultExport) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check should be superfluous as this.getDefaultExport throws when it cannot find an export. Therefore, the check if (syntheticExport) below is also unneeded and this can be simplified. Furthermore, the test for !this.exports[name] should always be true as otherwise, we would not get here.

syntheticExport = new SyntheticNamedExport(this.astContext, name, defaultExport);
this.syntheticExports.set(name, syntheticExport);
}
}
if (syntheticExport) {
return syntheticExport;
}
}

if (this.graph.shimMissingExports) {
this.shimMissingExport(name);
return this.exportShimVariable;
}
}
return undefined as any;
}
Expand Down Expand Up @@ -534,6 +577,7 @@ export default class Module {
originalSourcemap,
resolvedIds,
sourcemapChain,
syntheticNamedExports,
transformDependencies,
transformFiles
}: TransformModuleJSON & {
Expand All @@ -551,6 +595,9 @@ export default class Module {
if (typeof moduleSideEffects === 'boolean') {
this.moduleSideEffects = moduleSideEffects;
}
if (typeof syntheticNamedExports === 'boolean') {
this.syntheticNamedExports = syntheticNamedExports;
}

timeStart('generate ast', 3);

Expand Down Expand Up @@ -633,6 +680,7 @@ export default class Module {
originalSourcemap: this.originalSourcemap,
resolvedIds: this.resolvedIds,
sourcemapChain: this.sourcemapChain,
syntheticNamedExports: this.syntheticNamedExports,
transformDependencies: this.transformDependencies,
transformFiles: this.transformFiles
};
Expand Down
70 changes: 58 additions & 12 deletions src/ModuleLoader.ts
Expand Up @@ -17,6 +17,7 @@ import {
errBadLoader,
errCannotAssignModuleToChunk,
errEntryCannotBeExternal,
errExternalSyntheticExports,
errInternalIdCannotBeExternal,
errInvalidOption,
errNamespaceConflict,
Expand Down Expand Up @@ -243,7 +244,7 @@ export class ModuleLoader {
module.id,
(module.resolvedIds[source] =
module.resolvedIds[source] ||
this.handleMissingImports(await this.resolveId(source, module.id), source, module.id))
this.handleResolveId(await this.resolveId(source, module.id), source, module.id))
)
) as Promise<unknown>[]),
...module.getDynamicImportExpressions().map((specifier, index) =>
Expand Down Expand Up @@ -272,6 +273,7 @@ export class ModuleLoader {
id: string,
importer: string,
moduleSideEffects: boolean,
syntheticNamedExports: boolean,
isEntry: boolean
): Promise<Module> {
const existingModule = this.modulesById.get(id);
Expand All @@ -280,7 +282,13 @@ export class ModuleLoader {
return Promise.resolve(existingModule);
}

const module: Module = new Module(this.graph, id, moduleSideEffects, isEntry);
const module: Module = new Module(
this.graph,
id,
moduleSideEffects,
syntheticNamedExports,
isEntry
);
this.modulesById.set(id, module);
this.graph.watchFiles[id] = true;
const manualChunkAlias = this.getManualChunk(id);
Expand Down Expand Up @@ -321,6 +329,9 @@ export class ModuleLoader {
if (typeof sourceDescription.moduleSideEffects === 'boolean') {
module.moduleSideEffects = sourceDescription.moduleSideEffects;
}
if (typeof sourceDescription.syntheticNamedExports === 'boolean') {
module.syntheticNamedExports = sourceDescription.syntheticNamedExports;
manucorporat marked this conversation as resolved.
Show resolved Hide resolved
}
return transform(this.graph, sourceDescription, module);
})
.then((source: TransformModuleJSON | ModuleJSON) => {
Expand Down Expand Up @@ -370,7 +381,13 @@ export class ModuleLoader {
}
return Promise.resolve(externalModule);
} else {
return this.fetchModule(resolvedId.id, importer, resolvedId.moduleSideEffects, false);
return this.fetchModule(
resolvedId.id,
importer,
resolvedId.moduleSideEffects,
resolvedId.syntheticNamedExports,
false
);
}
}

Expand All @@ -387,7 +404,35 @@ export class ModuleLoader {
return {
external: true,
id: source,
moduleSideEffects: this.hasModuleSideEffects(source, true)
moduleSideEffects: this.hasModuleSideEffects(source, true),
syntheticNamedExports: false
};
}
return resolvedId;
}

private handleResolveId(
resolvedId: ResolvedId | null,
source: string,
importer: string
): ResolvedId {
return this.handleSyntheticExports(
this.handleMissingImports(resolvedId, source, importer),
source,
importer
);
}

private handleSyntheticExports(
manucorporat marked this conversation as resolved.
Show resolved Hide resolved
resolvedId: ResolvedId,
source: string,
importer: string
): ResolvedId {
if (resolvedId.external && resolvedId.syntheticNamedExports) {
this.graph.warn(errExternalSyntheticExports(source, importer));
return {
...resolvedId,
syntheticNamedExports: false
};
}
return resolvedId;
Expand All @@ -407,7 +452,7 @@ export class ModuleLoader {
: resolveIdResult;

if (typeof id === 'string') {
return this.fetchModule(id, undefined as any, true, isEntry);
return this.fetchModule(id, undefined as any, true, false, isEntry);
}
return error(errUnresolvedEntry(unresolvedId));
});
Expand All @@ -420,6 +465,7 @@ export class ModuleLoader {
let id = '';
let external = false;
let moduleSideEffects = null;
let syntheticNamedExports = false;
if (resolveIdResult) {
if (typeof resolveIdResult === 'object') {
id = resolveIdResult.id;
Expand All @@ -429,6 +475,9 @@ export class ModuleLoader {
if (typeof resolveIdResult.moduleSideEffects === 'boolean') {
moduleSideEffects = resolveIdResult.moduleSideEffects;
}
if (typeof resolveIdResult.syntheticNamedExports === 'boolean') {
syntheticNamedExports = resolveIdResult.syntheticNamedExports;
}
} else {
if (this.isExternal(resolveIdResult, importer, true)) {
external = true;
Expand All @@ -448,7 +497,8 @@ export class ModuleLoader {
moduleSideEffects:
typeof moduleSideEffects === 'boolean'
? moduleSideEffects
: this.hasModuleSideEffects(id, external)
: this.hasModuleSideEffects(id, external),
syntheticNamedExports
};
}

Expand Down Expand Up @@ -478,13 +528,9 @@ export class ModuleLoader {
if (resolution == null) {
return (module.resolvedIds[specifier] =
module.resolvedIds[specifier] ||
this.handleMissingImports(
await this.resolveId(specifier, module.id),
specifier,
module.id
));
this.handleResolveId(await this.resolveId(specifier, module.id), specifier, module.id));
}
return this.handleMissingImports(
return this.handleResolveId(
this.normalizeResolveIdResult(resolution, importer, specifier),
specifier,
importer
Expand Down
17 changes: 13 additions & 4 deletions src/ast/variables/NamespaceVariable.ts
Expand Up @@ -15,11 +15,13 @@ export default class NamespaceVariable extends Variable {
private containsExternalNamespace = false;
private referencedEarly = false;
private references: Identifier[] = [];
private syntheticNamedExports: boolean;

constructor(context: AstContext) {
constructor(context: AstContext, syntheticNamedExports: boolean) {
super(context.getModuleName());
this.context = context;
this.module = context.module;
this.syntheticNamedExports = syntheticNamedExports;
}

addReference(identifier: Identifier) {
Expand Down Expand Up @@ -99,10 +101,17 @@ export default class NamespaceVariable extends Variable {
}

const name = this.getName();
const defaultExport = this.syntheticNamedExports ? this.module.getDefaultExport() : undefined;

const callee = options.freeze ? `/*#__PURE__*/Object.freeze` : '';
const membersStr = members.join(`,${n}`);
let output = `${options.varOrConst} ${name}${_}=${_}${callee}({${n}${membersStr}${n}});`;
let output = `{${n}${members.join(`,${n}`)}${n}}`;
if (defaultExport) {
manucorporat marked this conversation as resolved.
Show resolved Hide resolved
output = `/*#__PURE__*/Object.assign(${output}, ${defaultExport.getName()})`;
}
if (options.freeze) {
output = `/*#__PURE__*/Object.freeze(${output})`;
}

output = `${options.varOrConst} ${name}${_}=${_}${output};`;

if (options.format === 'system' && this.exportName) {
output += `${n}exports('${this.exportName}',${_}${name});`;
Expand Down
25 changes: 25 additions & 0 deletions src/ast/variables/SyntheticNamedExport.ts
@@ -0,0 +1,25 @@
import Module, { AstContext } from '../../Module';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor thing: Maybe call this a ...Variable so that it fits the names of the other variable types

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

import { InclusionContext } from '../ExecutionContext';
import ExportDefaultVariable from './ExportDefaultVariable';
import Variable from './Variable';

export default class SyntheticNamedExport extends Variable {
context: AstContext;
defaultVariable: ExportDefaultVariable;
module: Module;

constructor(context: AstContext, name: string, defaultVariable: ExportDefaultVariable) {
super(name);
this.context = context;
this.module = context.module;
this.defaultVariable = defaultVariable;
this.setRenderNames(defaultVariable.getName(), name);
}

include(context: InclusionContext) {
if (!this.included) {
this.included = true;
this.context.includeVariable(context, this.defaultVariable);
manucorporat marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
6 changes: 5 additions & 1 deletion src/ast/variables/Variable.ts
Expand Up @@ -49,7 +49,7 @@ export default class Variable implements ExpressionEntity {

getName(): string {
const name = this.renderName || this.name;
return this.renderBaseName ? `${this.renderBaseName}.${name}` : name;
return this.renderBaseName ? `${this.renderBaseName}${getPropertyAccess(name)}` : name;
}

getReturnExpressionWhenCalledAtPath(
Expand Down Expand Up @@ -107,3 +107,7 @@ export default class Variable implements ExpressionEntity {
return this.name;
}
}

const getPropertyAccess = (name: string) => {
return /^(?!\d)[\w$]+$/.test(name) ? `.${name}` : `[${JSON.stringify(name)}]`;
};