Impossible case in GetModuleNamespace?

[GeorgNeis]

In its step 3.c.ii, GetModuleNamespace throws a SyntaxError if the result of ResolveExport is null. I don't see how this case can ever happen, given how GetModuleNamespace is used in ModuleDeclarationInstantiation.

Here's my reasoning: If this ResolveExport returns null, one of the indirect exports in one of the (transitively) imported modules must be unresolvable. But this would have already been discovered earlier in ModuleDeclarationInstantiation, namely in step 9 or, recursively, in step 8.

Please correct me if I'm wrong.

If indeed this can never happen, then one obvious simplification is to just remove this case or replace it with an assert. But I'd like to propose a different simplification:

1. In GetExportedNames, remove the special handling of "default" (7.c.i).
2. This means that ResolveExport in GetModuleNamespace now can return null, namely exactly when name is "default" and the module does not provide a default export (add an assert to that effect). In that
   case, now do nothing (i.e. don't throw and don't add "default" to unambiguousNames).

The point of this simplification is to get rid of the duplicated special handling of "default".

[ajklein]

@allenwb @dherman who ought to be able to confirm @GeorgNeis's analysis.

[ajklein]

@allenwb @dherman who ought to be able to confirm @GeorgNeis's analysis.

[leobalter]

In its step 3.c.ii, GetModuleNamespace throws a SyntaxError if the result of ResolveExport is null. I don't see how this case can ever happen, given how GetModuleNamespace is used in ModuleDeclarationInstantiation.

@GeorgNeis we have a test for it on test262.

You can see this first file:

import * as ns from './instn-star-err-not-found-faulty_FIXTURE.js';

Importing from this second file:

export { x } from './instn-star-err-not-found-empty_FIXTURE.js';

Importing from this third file with single ;:

;

The first module is supposed to trigger this specific error. Please help me fixing it in case test262 is wrong.

[leobalter]

In its step 3.c.ii, GetModuleNamespace throws a SyntaxError if the result of ResolveExport is null. I don't see how this case can ever happen, given how GetModuleNamespace is used in ModuleDeclarationInstantiation.

@GeorgNeis we have a test for it on test262.

You can see this first file:

import * as ns from './instn-star-err-not-found-faulty_FIXTURE.js';

Importing from this second file:

export { x } from './instn-star-err-not-found-empty_FIXTURE.js';

Importing from this third file with single ;:

;

The first module is supposed to trigger this specific error. Please help me fixing it in case test262 is wrong.

[ajklein]

@leobalter But that would get an error at step 9.b of ModuleDeclarationInstantiation on './instn-star-err-not-found-empty_FIXTURE.js', since it will try to resolve the export { x } from ... and fail. This happens before we get to GetModuleNamespace (the first module is still at step 8.c waiting on the instantiation of the second module at this point).

[ajklein]

@leobalter But that would get an error at step 9.b of ModuleDeclarationInstantiation on './instn-star-err-not-found-empty_FIXTURE.js', since it will try to resolve the export { x } from ... and fail. This happens before we get to GetModuleNamespace (the first module is still at step 8.c waiting on the instantiation of the second module at this point).

[leobalter]

cc @jugglinmike

[leobalter]

cc @jugglinmike

[ajklein]

@bterlson Would love your input here; if @GeorgNeis is right, then this would make for a nice simplification of the spec.

[ajklein]

@bterlson Would love your input here; if @GeorgNeis is right, then this would make for a nice simplification of the spec.

[allenwb]

Questions like this have come up before, and I have a general answer: The module that is passed into this abstract operation is only required to be some concrete subclass of Module Record. It is not necessarily a Source Text Module Record. But it might be some other kind of specialized concrete Module Record provided by an implementation defined extension or by some future specification (for example, the module record for a dynamic module or a Common JS module). Because we don't know the specific module kind, we can't fully analyze the behavior of module.ResolveExport or for that matter module.ModuleDeclarationInstantiation. They are black boxes and hence the spec has to be prepared for ResolveExport to fail in ways that would not occur if we were sure that module was always gong to be a Source Text Module Record.

Of course, if an actual implementation only supports Source Text Module Records, then Georg's analysis would be correct and the implementation could skip testings for those occurrences.

[allenwb]

Questions like this have come up before, and I have a general answer: The module that is passed into this abstract operation is only required to be some concrete subclass of Module Record. It is not necessarily a Source Text Module Record. But it might be some other kind of specialized concrete Module Record provided by an implementation defined extension or by some future specification (for example, the module record for a dynamic module or a Common JS module). Because we don't know the specific module kind, we can't fully analyze the behavior of module.ResolveExport or for that matter module.ModuleDeclarationInstantiation. They are black boxes and hence the spec has to be prepared for ResolveExport to fail in ways that would not occur if we were sure that module was always gong to be a Source Text Module Record.

Of course, if an actual implementation only supports Source Text Module Records, then Georg's analysis would be correct and the implementation could skip testings for those occurrences.

[littledan]

@allenwb Do you have concrete ideas about what other kinds of modules may break the kinds of invariants that Source Text Module Records tend to obey? This could be helpful as implementations proceed as well as in designing how modules fit into various embedding environments.

[littledan]

@allenwb Do you have concrete ideas about what other kinds of modules may break the kinds of invariants that Source Text Module Records tend to obey? This could be helpful as implementations proceed as well as in designing how modules fit into various embedding environments.

[allenwb]

@littledan
Not really, but what if the module we are trying to get the namespace of is some sort of API constructed dynamic module with distinct mechanism for setting the list of exported names and for resolving individual exported names. Perhaps the exported name resolution is done by a call back to user code. Then each callback to resolve a name could do anything, including sometimes returning null and other times doing something else. In cases like this, we can't enforce that the observed behavior is rational, but at least specify error checks such that behavior is the same across multiple implementations

Avoiding such situations would require additional requirements in the spec. For example we could specify a bunch of invariants relating to the various fields of Table 39. But it is hard to think of what all the necessary invariants might be, particularly if we are allowing for additional host defined types of modules. It seems easier to simply have situational checks such as the one in 3.c.ii that deal with possible inconsistencies in context. Also, stating such invariants, leave unspecified where in the processing of a module the checking of the invariant is performed and how violations are reported.

Alternatively, we could replace 3.c.ii with an assertion. But that's really just another way to list an invariant.

[allenwb]

@littledan
Not really, but what if the module we are trying to get the namespace of is some sort of API constructed dynamic module with distinct mechanism for setting the list of exported names and for resolving individual exported names. Perhaps the exported name resolution is done by a call back to user code. Then each callback to resolve a name could do anything, including sometimes returning null and other times doing something else. In cases like this, we can't enforce that the observed behavior is rational, but at least specify error checks such that behavior is the same across multiple implementations

Avoiding such situations would require additional requirements in the spec. For example we could specify a bunch of invariants relating to the various fields of Table 39. But it is hard to think of what all the necessary invariants might be, particularly if we are allowing for additional host defined types of modules. It seems easier to simply have situational checks such as the one in 3.c.ii that deal with possible inconsistencies in context. Also, stating such invariants, leave unspecified where in the processing of a module the checking of the invariant is performed and how violations are reported.

Alternatively, we could replace 3.c.ii with an assertion. But that's really just another way to list an invariant.

[GeorgNeis]

Turns out I was wrong. Simple example:

A: import "B"; export {x} from "A";
B: import * as a from "A";

Instantiation of A triggers instantiation of B, which triggers A.ResolveExport(x, {}), which returns null.

[GeorgNeis]

Turns out I was wrong. Simple example:

A: import "B"; export {x} from "A";
B: import * as a from "A";

Instantiation of A triggers instantiation of B, which triggers A.ResolveExport(x, {}), which returns null.