Skip to content

Commit

Permalink
feat: support external star exports on namespace objects (#3474)
Browse files Browse the repository at this point in the history 
* feat: support external star exports on namespace objects

* fixup test description, form test

* fixup form tests

* fixup Object.create assignment, __PURE__ tweaks

* remove invalid test changes

* Add pure annotations again, small refactor

* Simplify namespace handling logic and extend tests

* Improve coverage

Co-authored-by: Lukas Taegert-Atkinson <lukas.taegert-atkinson@tngtech.com>
  • Loading branch information
@guybedford @lukastaegert
guybedford and lukastaegert committed Apr 4, 2020
1 parent 722bc25 commit 74cec2e
Show file tree
Hide file tree
Showing 31 changed files with 269 additions and 71 deletions.
56 changes: 28 additions & 28 deletions src/Module.ts
View file
Expand Up @@ -100,9 +100,9 @@ export interface AstContext {
getModuleName: () => string;
getReexports: () => string[];
importDescriptions: { [name: string]: ImportDescription };
includeAndGetReexportedExternalNamespaces: () => ExternalVariable[];
includeDynamicImport: (node: ImportExpression) => void;
includeVariable: (context: InclusionContext, variable: Variable) => void;
isCrossChunkImport: (importDescription: ImportDescription) => boolean;
magicString: MagicString;
module: Module; // not to be used for tree-shaking
moduleContext: string;
Expand Down Expand Up @@ -198,7 +198,6 @@ export default class Module {
chunkName: string | null = null;
code!: string;
comments: CommentDescription[] = [];
customTransformCache!: boolean;
dependencies = new Set<Module | ExternalModule>();
dynamicallyImportedBy: Module[] = [];
dynamicDependencies = new Set<Module | ExternalModule>();
Expand All @@ -208,11 +207,9 @@ export default class Module {
}[] = [];
excludeFromSourcemap: boolean;
execIndex = Infinity;
exportAllModules: (Module | ExternalModule)[] = [];
exportAllSources = new Set<string>();
exports: { [name: string]: ExportDescription } = Object.create(null);
exportsAll: { [name: string]: string } = Object.create(null);
exportShimVariable: ExportShimVariable = new ExportShimVariable(this);
facadeChunk: Chunk | null = null;
id: string;
importDescriptions: { [name: string]: ImportDescription } = Object.create(null);
Expand Down Expand Up @@ -240,8 +237,11 @@ export default class Module {
private ast!: Program;
private astContext!: AstContext;
private context: string;
private customTransformCache!: boolean;
private defaultExport: ExportDefaultVariable | null | undefined = null;
private esTreeAst!: acorn.Node;
private exportAllModules: (Module | ExternalModule)[] = [];
private exportShimVariable: ExportShimVariable = new ExportShimVariable(this);
private graph: Graph;
private magicString!: MagicString;
private namespaceVariable: NamespaceVariable | null = null;
Expand Down Expand Up @@ -700,10 +700,11 @@ export default class Module {
getModuleName: this.basename.bind(this),
getReexports: this.getReexports.bind(this),
importDescriptions: this.importDescriptions,
includeAndGetReexportedExternalNamespaces: this.includeAndGetReexportedExternalNamespaces.bind(
this
),
includeDynamicImport: this.includeDynamicImport.bind(this),
includeVariable: this.includeVariable.bind(this),
isCrossChunkImport: importDescription =>
(importDescription.module as Module).chunk !== this.chunk,
magicString: this.magicString,
module: this,
moduleContext: this.context,
Expand Down Expand Up @@ -864,18 +865,6 @@ export default class Module {
const source = node.source.value;
this.sources.add(source);
for (const specifier of node.specifiers) {
const localName = specifier.local.name;

if (this.importDescriptions[localName]) {
return this.error(
{
code: 'DUPLICATE_IMPORT',
message: `Duplicated import '${localName}'`
},
specifier.start
);
}

const isDefault = specifier.type === NodeType.ImportDefaultSpecifier;
const isNamespace = specifier.type === NodeType.ImportNamespaceSpecifier;

Expand All @@ -884,7 +873,7 @@ export default class Module {
: isNamespace
? '*'
: (specifier as ImportSpecifier).imported.name;
this.importDescriptions[localName] = {
this.importDescriptions[specifier.local.name] = {
module: null as any, // filled in later
name,
source,
Expand All @@ -907,6 +896,19 @@ export default class Module {
}
}

private includeAndGetReexportedExternalNamespaces(): ExternalVariable[] {
const reexportedExternalNamespaces: ExternalVariable[] = [];
for (const module of this.exportAllModules) {
if (module instanceof ExternalModule) {
const externalVariable = module.getVariableForExportName('*');
externalVariable.include();
this.imports.add(externalVariable);
reexportedExternalNamespaces.push(externalVariable);
}
}
return reexportedExternalNamespaces;
}

private includeDynamicImport(node: ImportExpression) {
const resolution = (this.dynamicImports.find(dynamicImport => dynamicImport.node === node) as {
resolution: string | Module | ExternalModule | undefined;
Expand All @@ -929,14 +931,12 @@ export default class Module {
}

private shimMissingExport(name: string): void {
if (!this.exports[name]) {
this.graph.warn({
code: 'SHIMMED_EXPORT',
exporter: relativeId(this.id),
exportName: name,
message: `Missing export "${name}" has been shimmed in module ${relativeId(this.id)}.`
});
this.exports[name] = MISSING_EXPORT_SHIM_DESCRIPTION;
}
this.graph.warn({
code: 'SHIMMED_EXPORT',
exporter: relativeId(this.id),
exportName: name,
message: `Missing export "${name}" has been shimmed in module ${relativeId(this.id)}.`
});
this.exports[name] = MISSING_EXPORT_SHIM_DESCRIPTION;
}
}
37 changes: 22 additions & 15 deletions src/ast/variables/NamespaceVariable.ts
View file
Expand Up @@ -4,6 +4,7 @@ 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 {
Expand All @@ -12,7 +13,7 @@ export default class NamespaceVariable extends Variable {
memberVariables: { [name: string]: Variable } = Object.create(null);
module: Module;

private containsExternalNamespace = false;
private reexportedExternalNamespaces: ExternalVariable[] = [];
private referencedEarly = false;
private references: Identifier[] = [];
private syntheticNamedExports: boolean;
Expand Down Expand Up @@ -41,20 +42,14 @@ 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;
}
}
this.reexportedExternalNamespaces = this.context.includeAndGetReexportedExternalNamespaces();
if (this.context.preserveModules) {
for (const memberName of Object.keys(this.memberVariables))
this.memberVariables[memberName].include(context);
Expand All @@ -67,8 +62,9 @@ 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] !== '*') {
this.memberVariables[name] = this.context.traceExport(name);
}
}
}

Expand All @@ -91,21 +87,32 @@ export default class NamespaceVariable extends Variable {
return `${t}${safeName}: ${original.getName()}`;
});

members.unshift(`${t}__proto__:${_}null`);

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

const name = this.getName();
const hasExternalReexports = this.reexportedExternalNamespaces.length > 0;
if (!hasExternalReexports) members.unshift(`${t}__proto__:${_}null`);

let output = `{${n}${members.join(`,${n}`)}${n}}`;
if (this.syntheticNamedExports) {
output = `/*#__PURE__*/Object.assign(${output}, ${this.module.getDefaultExport().getName()})`;
if (hasExternalReexports || this.syntheticNamedExports) {
const assignmentArgs = members.length > 0 ? [output] : [];
if (hasExternalReexports) {
assignmentArgs.unshift(
'/*#__PURE__*/Object.create(null)',
...this.reexportedExternalNamespaces.map(variable => variable.getName())
);
}
if (this.syntheticNamedExports) {
assignmentArgs.push(this.module.getDefaultExport().getName());
}
output = `/*#__PURE__*/Object.assign(${assignmentArgs.join(`,${_}`)})`;

This comment has been minimized.

Sign in to view

Copy link
@dasa

dasa Aug 13, 2020

Is there a way to avoid this Object.assign since it doesn't work in IE 11?

This comment has been minimized.

Sign in to view

Copy link
@shellscape

shellscape Aug 13, 2020

Contributor

I'd recommend a polyfill. Hardly any major payer supports ie11.
}
if (options.freeze) {
output = `/*#__PURE__*/Object.freeze(${output})`;
}

const name = this.getName();
output = `${options.varOrConst} ${name}${_}=${_}${output};`;

if (options.format === 'system' && this.exportName) {
Expand Down
4 changes: 2 additions & 2 deletions test/form/samples/compact/_expected/amd.js
View file
@@ -1,6 +1,6 @@
define(['external'],function(x){'use strict';x=x&&Object.prototype.hasOwnProperty.call(x,'default')?x['default']:x;var self=/*#__PURE__*/Object.freeze({[Symbol.toStringTag]:'Module',__proto__:null,get default(){return foo}});console.log(self);
define(['external'],function(x){'use strict';x=x&&Object.prototype.hasOwnProperty.call(x,'default')?x['default']:x;var self=/*#__PURE__*/Object.freeze({__proto__:null,[Symbol.toStringTag]:'Module',get default(){return foo}});console.log(self);
function foo () {
console.log( x );
}
// trailing comment
return foo;});
return foo;});
2 changes: 1 addition & 1 deletion test/form/samples/compact/_expected/cjs.js
View file
@@ -1,4 +1,4 @@
'use strict';function _interopDefault(e){return(e&&(typeof e==='object')&&'default'in e)?e['default']:e}var x=_interopDefault(require('external'));var self=/*#__PURE__*/Object.freeze({[Symbol.toStringTag]:'Module',__proto__:null,get default(){return foo}});console.log(self);
'use strict';function _interopDefault(e){return(e&&(typeof e==='object')&&'default'in e)?e['default']:e}var x=_interopDefault(require('external'));var self=/*#__PURE__*/Object.freeze({__proto__:null,[Symbol.toStringTag]:'Module',get default(){return foo}});console.log(self);
function foo () {
console.log( x );
}
Expand Down
2 changes: 1 addition & 1 deletion test/form/samples/compact/_expected/es.js
View file
@@ -1,4 +1,4 @@
import x from'external';var self=/*#__PURE__*/Object.freeze({[Symbol.toStringTag]:'Module',__proto__:null,get default(){return foo}});console.log(self);
import x from'external';var self=/*#__PURE__*/Object.freeze({__proto__:null,[Symbol.toStringTag]:'Module',get default(){return foo}});console.log(self);
function foo () {
console.log( x );
}
Expand Down
4 changes: 2 additions & 2 deletions test/form/samples/compact/_expected/iife.js
View file
@@ -1,6 +1,6 @@
var foo=(function(x){'use strict';x=x&&Object.prototype.hasOwnProperty.call(x,'default')?x['default']:x;var self=/*#__PURE__*/Object.freeze({[Symbol.toStringTag]:'Module',__proto__:null,get default(){return foo}});console.log(self);
var foo=(function(x){'use strict';x=x&&Object.prototype.hasOwnProperty.call(x,'default')?x['default']:x;var self=/*#__PURE__*/Object.freeze({__proto__:null,[Symbol.toStringTag]:'Module',get default(){return foo}});console.log(self);
function foo () {
console.log( x );
}
// trailing comment
return foo;}(x));
return foo;}(x));
2 changes: 1 addition & 1 deletion test/form/samples/compact/_expected/system.js
View file
@@ -1,4 +1,4 @@
System.register('foo',['external'],function(exports){'use strict';var x;return{setters:[function(module){x=module.default;}],execute:function(){exports('default',foo);var self=/*#__PURE__*/Object.freeze({[Symbol.toStringTag]:'Module',__proto__:null,get default(){return foo}});console.log(self);
System.register('foo',['external'],function(exports){'use strict';var x;return{setters:[function(module){x=module.default;}],execute:function(){exports('default',foo);var self=/*#__PURE__*/Object.freeze({__proto__:null,[Symbol.toStringTag]:'Module',get default(){return foo}});console.log(self);
function foo () {
console.log( x );
}
Expand Down
4 changes: 2 additions & 2 deletions test/form/samples/compact/_expected/umd.js
View file
@@ -1,6 +1,6 @@
(function(g,f){typeof exports==='object'&&typeof module!=='undefined'?module.exports=f(require('external')):typeof define==='function'&&define.amd?define(['external'],f):(g=g||self,g.foo=f(g.x));}(this,(function(x){'use strict';x=x&&Object.prototype.hasOwnProperty.call(x,'default')?x['default']:x;var self=/*#__PURE__*/Object.freeze({[Symbol.toStringTag]:'Module',__proto__:null,get default(){return foo}});console.log(self);
(function(g,f){typeof exports==='object'&&typeof module!=='undefined'?module.exports=f(require('external')):typeof define==='function'&&define.amd?define(['external'],f):(g=g||self,g.foo=f(g.x));}(this,(function(x){'use strict';x=x&&Object.prototype.hasOwnProperty.call(x,'default')?x['default']:x;var self=/*#__PURE__*/Object.freeze({__proto__:null,[Symbol.toStringTag]:'Module',get default(){return foo}});console.log(self);
function foo () {
console.log( x );
}
// trailing comment
return foo;})));
return foo;})));
2 changes: 1 addition & 1 deletion test/form/samples/namespace-tostringtag/_expected/amd.js
View file
@@ -1,8 +1,8 @@
define(['exports'], function (exports) { 'use strict';

var self = /*#__PURE__*/Object.freeze({
[Symbol.toStringTag]: 'Module',
__proto__: null,
[Symbol.toStringTag]: 'Module',
get p () { return p; }
});

Expand Down
2 changes: 1 addition & 1 deletion test/form/samples/namespace-tostringtag/_expected/cjs.js
View file
Expand Up @@ -3,8 +3,8 @@
Object.defineProperty(exports, '__esModule', { value: true });

var self = /*#__PURE__*/Object.freeze({
[Symbol.toStringTag]: 'Module',
__proto__: null,
[Symbol.toStringTag]: 'Module',
get p () { return p; }
});

Expand Down
2 changes: 1 addition & 1 deletion test/form/samples/namespace-tostringtag/_expected/es.js
View file
@@ -1,6 +1,6 @@
var self = /*#__PURE__*/Object.freeze({
[Symbol.toStringTag]: 'Module',
__proto__: null,
[Symbol.toStringTag]: 'Module',
get p () { return p; }
});

Expand Down
2 changes: 1 addition & 1 deletion test/form/samples/namespace-tostringtag/_expected/iife.js
View file
Expand Up @@ -2,8 +2,8 @@ var iife = (function (exports) {
'use strict';

var self = /*#__PURE__*/Object.freeze({
[Symbol.toStringTag]: 'Module',
__proto__: null,
[Symbol.toStringTag]: 'Module',
get p () { return p; }
});

Expand Down
2 changes: 1 addition & 1 deletion test/form/samples/namespace-tostringtag/_expected/system.js
View file
Expand Up @@ -4,8 +4,8 @@ System.register('iife', [], function (exports) {
execute: function () {

var self = /*#__PURE__*/Object.freeze({
[Symbol.toStringTag]: 'Module',
__proto__: null,
[Symbol.toStringTag]: 'Module',
get p () { return p; }
});

Expand Down
2 changes: 1 addition & 1 deletion test/form/samples/namespace-tostringtag/_expected/umd.js
View file
Expand Up @@ -5,8 +5,8 @@
}(this, (function (exports) { 'use strict';

var self = /*#__PURE__*/Object.freeze({
[Symbol.toStringTag]: 'Module',
__proto__: null,
[Symbol.toStringTag]: 'Module',
get p () { return p; }
});

Expand Down
24 changes: 24 additions & 0 deletions test/form/samples/ns-external-star-reexport/_config.js
View file
@@ -0,0 +1,24 @@
module.exports = {
description: 'supports namespaces with external star reexports',
options: {
external: ['external1', 'external2'],
plugins: {
transform(code, id) {
if (id.endsWith('override.js')) {
return {
code,
syntheticNamedExports: true
};
}
return null;
}
},
output: {
globals: {
external1: 'external1',
external2: 'external2'
},
name: 'bundle'
}
}
};
21 changes: 21 additions & 0 deletions test/form/samples/ns-external-star-reexport/_expected/amd.js
View file
@@ -0,0 +1,21 @@
define(['exports', 'external1', 'external2'], function (exports, external1, external2) { 'use strict';

var reexportExternal = /*#__PURE__*/Object.freeze(/*#__PURE__*/Object.assign(/*#__PURE__*/Object.create(null), external1));

const extra = 'extra';

const override = 'override';
var reexportExternalsWithOverride = { synthetic: 'synthetic' };

var reexportExternalsWithOverride$1 = /*#__PURE__*/Object.freeze(/*#__PURE__*/Object.assign(/*#__PURE__*/Object.create(null), external1, external2, {
override: override,
'default': reexportExternalsWithOverride,
extra: extra
}, reexportExternalsWithOverride));

exports.external = reexportExternal;
exports.externalOverride = reexportExternalsWithOverride$1;

Object.defineProperty(exports, '__esModule', { value: true });

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

Object.defineProperty(exports, '__esModule', { value: true });

var external1 = require('external1');
var external2 = require('external2');

var reexportExternal = /*#__PURE__*/Object.freeze(/*#__PURE__*/Object.assign(/*#__PURE__*/Object.create(null), external1));

const extra = 'extra';

const override = 'override';
var reexportExternalsWithOverride = { synthetic: 'synthetic' };

var reexportExternalsWithOverride$1 = /*#__PURE__*/Object.freeze(/*#__PURE__*/Object.assign(/*#__PURE__*/Object.create(null), external1, external2, {
override: override,
'default': reexportExternalsWithOverride,
extra: extra
}, reexportExternalsWithOverride));

exports.external = reexportExternal;
exports.externalOverride = reexportExternalsWithOverride$1;

0 comments on commit 74cec2e

Please sign in to comment.