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: support external star exports on namespace objects #3474

Merged
merged 8 commits into from Apr 4, 2020
Merged
Show file tree
Hide file tree
Changes from 5 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
64 changes: 41 additions & 23 deletions src/Module.ts
Expand Up @@ -5,7 +5,7 @@ import extractAssignedNames from 'rollup-pluginutils/src/extractAssignedNames';
import {
createHasEffectsContext,
createInclusionContext,
InclusionContext
InclusionContext,
} from './ast/ExecutionContext';
import ExportAllDeclaration from './ast/nodes/ExportAllDeclaration';
import ExportDefaultDeclaration from './ast/nodes/ExportDefaultDeclaration';
Expand Down Expand Up @@ -42,7 +42,7 @@ import {
ResolvedIdMap,
RollupError,
RollupWarning,
TransformModuleJSON
TransformModuleJSON,
} from './rollup/types';
import { error, Errors } from './utils/error';
import getCodeFrame from './utils/getCodeFrame';
Expand Down Expand Up @@ -101,6 +101,7 @@ export interface AstContext {
getReexports: () => string[];
importDescriptions: { [name: string]: ImportDescription };
includeDynamicImport: (node: ImportExpression) => void;
includeExternalReexportNamespace: (name: string) => ExternalVariable;
includeVariable: (context: InclusionContext, variable: Variable) => void;
isCrossChunkImport: (importDescription: ImportDescription) => boolean;
magicString: MagicString;
Expand All @@ -122,7 +123,7 @@ export interface AstContext {
export const defaultAcornOptions: acorn.Options = {
ecmaVersion: 2020,
preserveParens: false,
sourceType: 'module'
sourceType: 'module',
};

function tryParse(module: Module, Parser: typeof acorn.Parser, acornOptions: acorn.Options) {
Expand All @@ -131,7 +132,7 @@ function tryParse(module: Module, Parser: typeof acorn.Parser, acornOptions: aco
...defaultAcornOptions,
...acornOptions,
onComment: (block: boolean, text: string, start: number, end: number) =>
module.comments.push({ block, text, start, end })
module.comments.push({ block, text, start, end }),
});
} catch (err) {
let message = err.message.replace(/ \(\d+:\d+\)$/, '');
Expand All @@ -144,7 +145,7 @@ function tryParse(module: Module, Parser: typeof acorn.Parser, acornOptions: aco
{
code: 'PARSE_ERROR',
message,
parserError: err
parserError: err,
},
err.pos
);
Expand All @@ -163,15 +164,15 @@ function handleMissingExport(
message: `'${exportName}' is not exported by ${relativeId(
importedModule
)}, imported by ${relativeId(importingModule.id)}`,
url: `https://rollupjs.org/guide/en/#error-name-is-not-exported-by-module`
url: `https://rollupjs.org/guide/en/#error-name-is-not-exported-by-module`,
},
importerStart!
);
}

const MISSING_EXPORT_SHIM_DESCRIPTION: ExportDescription = {
identifier: null,
localName: MISSING_EXPORT_SHIM_VARIABLE
localName: MISSING_EXPORT_SHIM_VARIABLE,
};

function getVariableForExportNameRecursive(
Expand Down Expand Up @@ -289,17 +290,17 @@ export default class Module {
loc: {
column: location.column,
file: this.id,
line: location.line
line: location.line,
},
message: `Error when using sourcemap for reporting an error: ${e.message}`,
pos
pos,
});
}

props.loc = {
column: location.column,
file: this.id,
line: location.line
line: location.line,
};
props.frame = getCodeFrame(this.originalCode, location.line, location.column);
}
Expand Down Expand Up @@ -341,7 +342,7 @@ export default class Module {
return error({
code: Errors.SYNTHETIC_NAMED_EXPORTS_NEED_DEFAULT,
id: this.id,
message: `Modules with 'syntheticNamedExports' need a default export.`
message: `Modules with 'syntheticNamedExports' need a default export.`,
});
}
return this.defaultExport;
Expand Down Expand Up @@ -631,7 +632,7 @@ export default class Module {
sourcemapChain,
syntheticNamedExports,
transformDependencies,
transformFiles
transformFiles,
}: TransformModuleJSON & {
alwaysRemovedCode?: [number, number][];
transformFiles?: EmittedFile[] | undefined;
Expand Down Expand Up @@ -677,7 +678,7 @@ export default class Module {

this.magicString = new MagicString(code, {
filename: (this.excludeFromSourcemap ? null : fileName)!, // don't include plugin helpers in sourcemap
indentExclusionRanges: []
indentExclusionRanges: [],
});
for (const [start, end] of this.alwaysRemovedCode) {
this.magicString.remove(start, end);
Expand All @@ -701,8 +702,9 @@ export default class Module {
getReexports: this.getReexports.bind(this),
importDescriptions: this.importDescriptions,
includeDynamicImport: this.includeDynamicImport.bind(this),
includeExternalReexportNamespace: this.includeExternalReexportNamespace.bind(this),
includeVariable: this.includeVariable.bind(this),
isCrossChunkImport: importDescription =>
isCrossChunkImport: (importDescription) =>
(importDescription.module as Module).chunk !== this.chunk,
magicString: this.magicString,
module: this,
Expand All @@ -720,7 +722,7 @@ export default class Module {
this.graph.treeshakingOptions.unknownGlobalSideEffects)!,
usesTopLevelAwait: false,
warn: this.warn.bind(this),
warnDeprecation: this.graph.warnDeprecation.bind(this.graph)
warnDeprecation: this.graph.warnDeprecation.bind(this.graph),
};

this.scope = new ModuleScope(this.graph.scope, this.astContext);
Expand All @@ -739,7 +741,7 @@ export default class Module {
ast: this.esTreeAst,
code: this.code,
customTransformCache: this.customTransformCache,
dependencies: Array.from(this.dependencies).map(module => module.id),
dependencies: Array.from(this.dependencies).map((module) => module.id),
id: this.id,
moduleSideEffects: this.moduleSideEffects,
originalCode: this.originalCode,
Expand All @@ -748,7 +750,7 @@ export default class Module {
sourcemapChain: this.sourcemapChain,
syntheticNamedExports: this.syntheticNamedExports,
transformDependencies: this.transformDependencies,
transformFiles: this.transformFiles
transformFiles: this.transformFiles,
};
}

Expand Down Expand Up @@ -809,7 +811,7 @@ export default class Module {

this.exports.default = {
identifier: node.variable.getAssignedVariableName(),
localName: 'default'
localName: 'default',
};
} else if (node instanceof ExportAllDeclaration) {
// export * from './other'
Expand All @@ -829,7 +831,7 @@ export default class Module {
specifier.type === NodeType.ExportNamespaceSpecifier ? '*' : specifier.local.name,
module: null as any, // filled in later,
source,
start: specifier.start
start: specifier.start,
};
}
} else if (node.declaration) {
Expand Down Expand Up @@ -870,7 +872,7 @@ export default class Module {
return this.error(
{
code: 'DUPLICATE_IMPORT',
message: `Duplicated import '${localName}'`
message: `Duplicated import '${localName}'`,
},
specifier.start
);
Expand All @@ -888,7 +890,7 @@ export default class Module {
module: null as any, // filled in later
name,
source,
start: specifier.start
start: specifier.start,
};
}
}
Expand All @@ -908,7 +910,9 @@ export default class Module {
}

private includeDynamicImport(node: ImportExpression) {
const resolution = (this.dynamicImports.find(dynamicImport => dynamicImport.node === node) as {
const resolution = (this.dynamicImports.find(
(dynamicImport) => dynamicImport.node === node
) as {
resolution: string | Module | ExternalModule | undefined;
}).resolution;
if (resolution instanceof Module) {
Expand All @@ -917,6 +921,20 @@ export default class Module {
}
}

private includeExternalReexportNamespace(name: string): ExternalVariable {
for (const module of this.exportAllModules) {
if (module instanceof ExternalModule) {
if (module.id === name) {
const externalVariable = module.getVariableForExportName('*');
externalVariable.include();
this.imports.add(externalVariable);
return externalVariable;
}
}
}
throw new Error(`Unable to find star reexport ExternalModule instance for ${name}`);
}

private includeVariable(context: InclusionContext, variable: Variable) {
const variableModule = variable.module;
if (!variable.included) {
Expand All @@ -934,7 +952,7 @@ export default class Module {
code: 'SHIMMED_EXPORT',
exporter: relativeId(this.id),
exportName: name,
message: `Missing export "${name}" has been shimmed in module ${relativeId(this.id)}.`
message: `Missing export "${name}" has been shimmed in module ${relativeId(this.id)}.`,
});
this.exports[name] = MISSING_EXPORT_SHIM_DESCRIPTION;
}
Expand Down
38 changes: 24 additions & 14 deletions src/ast/variables/NamespaceVariable.ts
Expand Up @@ -4,15 +4,17 @@ import { RESERVED_NAMES } from '../../utils/reservedNames';
import { InclusionContext } from '../ExecutionContext';
import Identifier from '../nodes/Identifier';
import { UNKNOWN_PATH } from '../utils/PathTracker';
import ExternalVariable from './ExternalVariable';
import Variable from './Variable';

export default class NamespaceVariable extends Variable {
context: AstContext;
isNamespace!: true;
memberVariables: { [name: string]: Variable } = Object.create(null);
module: Module;
reexportedNamespaces: string[] = [];
private reexportedNamespaceVariables: ExternalVariable[] = [];

private containsExternalNamespace = false;
private referencedEarly = false;
private references: Identifier[] = [];
private syntheticNamedExports: boolean;
Expand Down Expand Up @@ -41,20 +43,16 @@ export default class NamespaceVariable extends Variable {

include(context: InclusionContext) {
if (!this.included) {
if (this.containsExternalNamespace) {
return this.context.error({
code: 'NAMESPACE_CANNOT_CONTAIN_EXTERNAL',
id: this.module.id,
message: `Cannot create an explicit namespace object for module "${this.context.getModuleName()}" because it contains a reexported external namespace`
});
}
this.included = true;
for (const identifier of this.references) {
if (identifier.context.getModuleExecIndex() <= this.context.getModuleExecIndex()) {
this.referencedEarly = true;
break;
}
}
for (const name of this.reexportedNamespaces) {
this.reexportedNamespaceVariables.push(this.context.includeExternalReexportNamespace(name));
}
if (this.context.preserveModules) {
for (const memberName of Object.keys(this.memberVariables))
this.memberVariables[memberName].include(context);
Expand All @@ -67,8 +65,11 @@ export default class NamespaceVariable extends Variable {

initialise() {
for (const name of this.context.getExports().concat(this.context.getReexports())) {
if (name[0] === '*' && name.length > 1) this.containsExternalNamespace = true;
this.memberVariables[name] = this.context.traceExport(name);
if (name[0] === '*' && name.length > 1) {
this.reexportedNamespaces.push(name.slice(1));
} else {
this.memberVariables[name] = this.context.traceExport(name);
}
}
}

Expand All @@ -77,7 +78,7 @@ export default class NamespaceVariable extends Variable {
const n = options.compact ? '' : '\n';
const t = options.indent;

const members = Object.keys(this.memberVariables).map(name => {
const members = Object.keys(this.memberVariables).map((name) => {
const original = this.memberVariables[name];

if (this.referencedEarly || original.isReassigned) {
Expand All @@ -91,16 +92,25 @@ export default class NamespaceVariable extends Variable {
return `${t}${safeName}: ${original.getName()}`;
});

members.unshift(`${t}__proto__:${_}null`);
const objectCreate = this.reexportedNamespaceVariables.length;
if (!objectCreate) members.unshift(`${t}__proto__:${_}null`);

if (options.namespaceToStringTag) {
members.unshift(`${t}[Symbol.toStringTag]:${_}'Module'`);
}

const name = this.getName();
let output = `{${n}${members.join(`,${n}`)}${n}}`;
if (this.syntheticNamedExports) {
output = `/*#__PURE__*/Object.assign(${output}, ${this.module.getDefaultExport().getName()})`;
if (this.reexportedNamespaceVariables.length) {
output = `Object.assign(Object.create(null), ${this.reexportedNamespaceVariables
.map((v) => v.getName())
.join(', ')}, ${output}${
this.syntheticNamedExports ? ', ' + this.module.getDefaultExport().getName() : ''
})`;
if (!options.freeze) output = '*#__PURE__*/' + output;
} else if (this.syntheticNamedExports) {
output = `Object.assign(${output}, ${this.module.getDefaultExport().getName()})`;
if (!options.freeze) output = '*#__PURE__*/' + output;
}
if (options.freeze) {
output = `/*#__PURE__*/Object.freeze(${output})`;
Expand Down
Expand Up @@ -6,7 +6,7 @@ define(function () { 'use strict';
};
const foo = 100;

var ns = /*#__PURE__*/Object.freeze(/*#__PURE__*/Object.assign({
var ns = /*#__PURE__*/Object.freeze(Object.assign({
__proto__: null,
foo: foo,
'default': d
Expand Down
Expand Up @@ -6,7 +6,7 @@ const d = {
};
const foo = 100;

var ns = /*#__PURE__*/Object.freeze(/*#__PURE__*/Object.assign({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also removed these "double pure comment" cases since I'm not sure that is necessary for the feature? Otherwise seems odd to potentially have three in a row.

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately all of them are necessary to have an effect. The annotations only annotate the call itself as pure but not its arguments. I readded the annotations and also slightly simplified the logic and merged it with the synthetic export logic.

var ns = /*#__PURE__*/Object.freeze(Object.assign({
__proto__: null,
foo: foo,
'default': d
Expand Down
Expand Up @@ -4,7 +4,7 @@ const d = {
};
const foo = 100;

var ns = /*#__PURE__*/Object.freeze(/*#__PURE__*/Object.assign({
var ns = /*#__PURE__*/Object.freeze(Object.assign({
__proto__: null,
foo: foo,
'default': d
Expand Down
Expand Up @@ -9,7 +9,7 @@ System.register([], function () {
};
const foo = 100;

var ns = /*#__PURE__*/Object.freeze(/*#__PURE__*/Object.assign({
var ns = /*#__PURE__*/Object.freeze(Object.assign({
__proto__: null,
foo: foo,
'default': d
Expand Down
7 changes: 7 additions & 0 deletions test/form/samples/ns-external-star-reexport/_config.js
@@ -0,0 +1,7 @@
module.exports = {
description: 'supports namespace external star reexports',
formats: ['amd', 'cjs', 'system', 'es'],
options: {
external: ['external-ns-1', 'external-ns-2'],
},
};
11 changes: 11 additions & 0 deletions test/form/samples/ns-external-star-reexport/_expected/amd.js
@@ -0,0 +1,11 @@
define(['external-ns-1', 'external-ns-2'], function (externalNs1, externalNs2) { 'use strict';

const val = 5;

var ns = /*#__PURE__*/Object.freeze(Object.assign(Object.create(null), externalNs1, externalNs2, {
val: val
}));

return ns;

});
12 changes: 12 additions & 0 deletions test/form/samples/ns-external-star-reexport/_expected/cjs.js
@@ -0,0 +1,12 @@
'use strict';

var externalNs1 = require('external-ns-1');
var externalNs2 = require('external-ns-2');

const val = 5;

var ns = /*#__PURE__*/Object.freeze(Object.assign(Object.create(null), externalNs1, externalNs2, {
val: val
}));

module.exports = ns;