Skip to content

Commit

Permalink
Provide better error when creating a namespace object containing a re…
Browse files Browse the repository at this point in the history
…exported external namespace (#2499)

* Provide better error when creating a namespace object containing a reexported external namespace

* Fix windows path error
  • Loading branch information
lukastaegert committed Oct 9, 2018
1 parent a3be30d commit 0707a10
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 9 deletions.
14 changes: 6 additions & 8 deletions src/Chunk.ts
Expand Up @@ -247,7 +247,10 @@ export default class Chunk {
const tracedExports: { variable: Variable; module: Module | ExternalModule }[] = [];
for (const [index, exportName] of entryExportEntries) {
const traced = this.traceExport(exportName, this.entryModule);
if (traced.variable && !traced.variable.included && !traced.variable.isExternal) {
if (
!traced ||
(traced.variable && !traced.variable.included && !traced.variable.isExternal)
) {
continue;
}
tracedExports[index] = traced;
Expand Down Expand Up @@ -332,8 +335,8 @@ export default class Chunk {
): {
variable: Variable;
module: Module | ExternalModule;
} {
if (name === '*' || module instanceof ExternalModule) {
} | void {
if (name[0] === '*' || module instanceof ExternalModule) {
return { variable: module.traceExport(name), module };
}

Expand Down Expand Up @@ -365,11 +368,6 @@ export default class Chunk {
return;
}

// external star exports
if (name[0] === '*') {
return { variable: undefined, module: this.graph.moduleById.get(name.substr(1)) };
}

// resolve known star exports
for (let i = 0; i < module.exportAllModules.length; i++) {
const exportAllModule = module.exportAllModules[i];
Expand Down
14 changes: 13 additions & 1 deletion src/ast/variables/NamespaceVariable.ts
Expand Up @@ -11,17 +11,19 @@ export default class NamespaceVariable extends Variable {
// Not initialised during construction
originals: { [name: string]: Variable } = Object.create(null);
needsNamespaceBlock: boolean = false;

// only to be used for chunk-to-chunk import tracing, use context for data manipulation
module: Module;
private referencedEarly: boolean = false;
private references: Identifier[] = [];
private containsExternalNamespace: boolean = false;

constructor(context: AstContext, module: Module) {
super(context.getModuleName());
this.context = context;
this.module = module;
for (const name of this.context.getExports().concat(this.context.getReexports())) {
if (name[0] === '*' && name.length > 1) this.containsExternalNamespace = true;
this.originals[name] = this.context.traceExport(name);
}
}
Expand All @@ -33,6 +35,16 @@ export default class NamespaceVariable extends Variable {

include() {
if (!this.included) {
if (this.containsExternalNamespace) {
this.context.error(
{
code: 'NAMESPACE_CANNOT_CONTAIN_EXTERNAL',
message: `Cannot create an explicit namespace object for module "${this.context.getModuleName()}" because it contains a reexported external namespace`,
id: this.module.id
},
undefined
);
}
this.context.includeNamespace();
this.included = true;
this.needsNamespaceBlock = true;
Expand Down
15 changes: 15 additions & 0 deletions test/function/samples/internal-reexports-from-external/_config.js
@@ -0,0 +1,15 @@
const path = require('path');

module.exports = {
description:
'fails with a helpful error if creating a namespace object containing a reexported external namespace',
options: {
external: ['external']
},
error: {
code: 'NAMESPACE_CANNOT_CONTAIN_EXTERNAL',
message:
'Cannot create an explicit namespace object for module "reexport" because it contains a reexported external namespace',
id: path.join(__dirname, '/reexport.js')
}
};
@@ -0,0 +1,3 @@
import * as namespace from './reexport.js';

console.log(namespace);
@@ -0,0 +1 @@
export * from 'external';

0 comments on commit 0707a10

Please sign in to comment.