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

Provide better error when creating a namespace object containing a reexported external namespace #2499

Merged
merged 3 commits into from Oct 9, 2018
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
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';