Explicit specification on the evaluation ordering

[devsnek]

Refs: nodejs/modules#81
Refs: https://github.com/rwaldron/tc39-notes/blob/master/es8/2017-11/nov-28.md#conclusionresolution-13

Conclusion from November discussion was that there was no need to lock down the rules for the evaluation of AbstractModuleRecord because no one would break it. Now we are here and some members of the Node.js Modules team are pushing for Node.js to perform out-of-band evaluation of certain sources for interop purposes.

[ljharb]

Note that the “needs consensus” label is for the spec text - the decision (to prohibit messing with the evaluation order) already has consensus.

[ljharb]

Note that the “needs consensus” label is for the spec text - the decision (to prohibit messing with the evaluation order) already has consensus.

[weswigham]

Calls to any given AbstractModuleRecord's Evaluate function are strictly ordered already - what you're asking be specified is forbidding what a host's Evaluate can do, which is odd, IMO (especially given its already specified idempotent behavior). I don't think you can actually do it without stepping on the toes of what is very explicitly host-defined behavior. But if you wanna go that way, I'd suggest trying to forbid re-entrancy into the runtime during the resolution and instantiation phases of module graph resolution by locking down the realm in some way (flipping a bit that the Evaluate method on a SourceTextModuleRecord or the global script equivalent check, is perhaps sufficient), effectively forbidding you from executing any js in the same realm while resolution and instantiation is in progress.

[weswigham]

Calls to any given AbstractModuleRecord's Evaluate function are strictly ordered already - what you're asking be specified is forbidding what a host's Evaluate can do, which is odd, IMO (especially given its already specified idempotent behavior). I don't think you can actually do it without stepping on the toes of what is very explicitly host-defined behavior. But if you wanna go that way, I'd suggest trying to forbid re-entrancy into the runtime during the resolution and instantiation phases of module graph resolution by locking down the realm in some way (flipping a bit that the Evaluate method on a SourceTextModuleRecord or the global script equivalent check, is perhaps sufficient), effectively forbidding you from executing any js in the same realm while resolution and instantiation is in progress.

[domenic]

I think I agree with @weswigham's analysis. Although I point out that the web already has this sort of reentrancy with code such as

const a = document.createElement("a");
a.href = "javascript:console.log(1)";
a.click();

and so such restrictions on what the host is allowed to do during script evaluation wouldn't be web-compatible.

[domenic]

I think I agree with @weswigham's analysis. Although I point out that the web already has this sort of reentrancy with code such as

const a = document.createElement("a");
a.href = "javascript:console.log(1)";
a.click();

and so such restrictions on what the host is allowed to do during script evaluation wouldn't be web-compatible.

[bmeck]

@weswigham I think documenting the desired/prohibited effects of calling Instantiate / Evaluate are different from the ordering issue. I am also not convinced about reentrancy needing to be limited during Evaluate, but seems to be in line with Instantiate maybe? That topic has not been discussed at committee, but creating another issue if you want to discuss those things seems fine.

It is my understanding that this issue is about hosts taking a module graph that performs a -> c -> d, a -> b and performing b then a -> c -> d or similar reordering to an unexpected evaluation order is what is being talked about in this issue. This gets rather tricky to phrase and is part of why just letting the spec stand was somewhat appealing. The phrasing will need to discuss parallel graphs and how order is expected to be preserved while parallel graphs are running.

edit: fixed/cleaned my graph example.

[bmeck]

@weswigham I think documenting the desired/prohibited effects of calling Instantiate / Evaluate are different from the ordering issue. I am also not convinced about reentrancy needing to be limited during Evaluate, but seems to be in line with Instantiate maybe? That topic has not been discussed at committee, but creating another issue if you want to discuss those things seems fine.

It is my understanding that this issue is about hosts taking a module graph that performs a -> c -> d, a -> b and performing b then a -> c -> d or similar reordering to an unexpected evaluation order is what is being talked about in this issue. This gets rather tricky to phrase and is part of why just letting the spec stand was somewhat appealing. The phrasing will need to discuss parallel graphs and how order is expected to be preserved while parallel graphs are running.

edit: fixed/cleaned my graph example.

[weswigham]

@bmeck I've never proposed a host reordering a module graph like that, however. It's pretty clear that Evaluate calls are well ordered. What's unspecified is Evaluate's semantics itself for non-esm. Which is what leaves the door open for a non-esm modules' Evaluate to return a preexecuted (ie, during its Instantiate implementation, which, beyond module record creation does not forbid such an implementation) and cached namespace, as that fulfills the idempotency requirement. The host doesn't mess with the graph (or the esm in it) at all - it's just the implementation of some non-esm AbstractModuleRecord. You're confusing arbitrary mucking with a specified algorithm with utilizing well-established extensibility points. The effect of doing so with, eg, a cjs module record would be that it's execution occurs prior to any esm record; however without forbidding that reentrancy during early graph building phases, there's no way to forbid a non-esm AbstractModuleRecord from doing such a thing. It follows all the rules that are laid out.

[weswigham]

@bmeck I've never proposed a host reordering a module graph like that, however. It's pretty clear that Evaluate calls are well ordered. What's unspecified is Evaluate's semantics itself for non-esm. Which is what leaves the door open for a non-esm modules' Evaluate to return a preexecuted (ie, during its Instantiate implementation, which, beyond module record creation does not forbid such an implementation) and cached namespace, as that fulfills the idempotency requirement. The host doesn't mess with the graph (or the esm in it) at all - it's just the implementation of some non-esm AbstractModuleRecord. You're confusing arbitrary mucking with a specified algorithm with utilizing well-established extensibility points. The effect of doing so with, eg, a cjs module record would be that it's execution occurs prior to any esm record; however without forbidding that reentrancy during early graph building phases, there's no way to forbid a non-esm AbstractModuleRecord from doing such a thing. It follows all the rules that are laid out.

[ljharb]

Then the issue is that we need to specify that ESM-ness is irrelevant for evaluation order for all abstract module records?

[ljharb]

Then the issue is that we need to specify that ESM-ness is irrelevant for evaluation order for all abstract module records?

[bmeck]

@weswigham I'm just stating that it seems that the discussion of Instantiate/Evaluate needs to be split out since that isn't what was discussed at TC39 previously but as you have stated can create situations that can be used to emulate something similar the hoisting behavior to an extent that was seen as unexpected in the previous discussion in November 2017. The notification of the import occurring is propagated to the host in HostResolveImportedModule and there could be extra wording about things here with regard to preventing the behavior of creating these hoisted evaluations outside of Module records and in the host itself. I think that has some consensus, but also does not have a clear wording. Phrasing that new Abstract Module Records should not do something when resolving and should wait for Instantiate or Evaluate seems possible, but relies on outside specifications adhering similar to the idempotentcy requirement.

[bmeck]

@weswigham I'm just stating that it seems that the discussion of Instantiate/Evaluate needs to be split out since that isn't what was discussed at TC39 previously but as you have stated can create situations that can be used to emulate something similar the hoisting behavior to an extent that was seen as unexpected in the previous discussion in November 2017. The notification of the import occurring is propagated to the host in HostResolveImportedModule and there could be extra wording about things here with regard to preventing the behavior of creating these hoisted evaluations outside of Module records and in the host itself. I think that has some consensus, but also does not have a clear wording. Phrasing that new Abstract Module Records should not do something when resolving and should wait for Instantiate or Evaluate seems possible, but relies on outside specifications adhering similar to the idempotentcy requirement.

[weswigham]

The notification of the import occurring is propagated to the host in HostResolveImportedModule

Afaik, HostResolveImportedModule just does specifier resolution and maps some specifier to some potentially uninstantiated (potentially already evaluated) module record? Did you mean to link somewhere else?

[weswigham]

The notification of the import occurring is propagated to the host in HostResolveImportedModule

Afaik, HostResolveImportedModule just does specifier resolution and maps some specifier to some potentially uninstantiated (potentially already evaluated) module record? Did you mean to link somewhere else?

[domenic]

Then the issue is that we need to specify that ESM-ness is irrelevant for evaluation order for all abstract module records?

That might be what some people are asking for, but I don't think it's reasonable to place restrictions on other module systems in that way. As I noted, it's web-incompatible.I misunderstood; you were only talking about module records

That is already specified. Module records are evaluated in a deterministic order given by the spec.

[domenic]

Then the issue is that we need to specify that ESM-ness is irrelevant for evaluation order for all abstract module records?

That might be what some people are asking for, but I don't think it's reasonable to place restrictions on other module systems in that way. As I noted, it's web-incompatible.I misunderstood; you were only talking about module records

That is already specified. Module records are evaluated in a deterministic order given by the spec.

[weswigham]

Right. It's just what evaluate actually does that's unspecified. Which by no means is required to include any real execution of code.

[weswigham]

Right. It's just what evaluate actually does that's unspecified. Which by no means is required to include any real execution of code.

[bmeck]

@weswigham I have no desire to really enforce what Evaluate or Instantiate do in this issue. I'm more concerned that the ordering of effects to the JS environment of importing a module are consistent and do not change by what type of module is being imported in this issue. In particular some things like:

import('make-global:x'); // only gets HostResolveImportedModule
global.x // side effects before Instantiate or Evaluate called

Seem unintentional and even undesirable for deterministic ordering of effects, as do the reordering concerns when changing module types. Using Instantiate and Evaluate to return a precomputed value does not seem problematic.

[bmeck]

@weswigham I have no desire to really enforce what Evaluate or Instantiate do in this issue. I'm more concerned that the ordering of effects to the JS environment of importing a module are consistent and do not change by what type of module is being imported in this issue. In particular some things like:

import('make-global:x'); // only gets HostResolveImportedModule
global.x // side effects before Instantiate or Evaluate called

Seem unintentional and even undesirable for deterministic ordering of effects, as do the reordering concerns when changing module types. Using Instantiate and Evaluate to return a precomputed value does not seem problematic.

[domenic]

I think locking down HostResolveImportedModule is a more concrete and actionable ask, but I don't think it corresponds to the discussion in the notes, nor to the OP.

I'm not opposed to opening a new discussion on it, but I'm also unsure of the value on adding it. I think we could brainstorm a thousand different weird and strange things hosts could do in the various HostX operations, and it's not super-productive to try to proscibe them.

[domenic]

I think locking down HostResolveImportedModule is a more concrete and actionable ask, but I don't think it corresponds to the discussion in the notes, nor to the OP.

I'm not opposed to opening a new discussion on it, but I'm also unsure of the value on adding it. I think we could brainstorm a thousand different weird and strange things hosts could do in the various HostX operations, and it's not super-productive to try to proscibe them.

[weswigham]

Which means what you actually want is to lock down the runtime environment while resolution is in progress so a random AbstractModuleRecord subtype cannot much with the runtime during earlier phases, yes? Because otherwise what you suggest isn't actually preventable. Take your example - say that "make-global:x" resolved to a node native module - said native module is free to make that global during it's init step (or maybe even statically via some crazy introspection mechanic), which should be executed during the instantiate phase of the walk, meaning modules execute before that import would be encountered are able to witness the global.

[weswigham]

Which means what you actually want is to lock down the runtime environment while resolution is in progress so a random AbstractModuleRecord subtype cannot much with the runtime during earlier phases, yes? Because otherwise what you suggest isn't actually preventable. Take your example - say that "make-global:x" resolved to a node native module - said native module is free to make that global during it's init step (or maybe even statically via some crazy introspection mechanic), which should be executed during the instantiate phase of the walk, meaning modules execute before that import would be encountered are able to witness the global.

[bmeck]

@weswigham I'm trying to understand this init step, don't have a clear picture of what that is. During the instantiate phase of the walk I would also be surprised by having side effects outside of your module record's namespace and only side effects like allocation of function declarations etc. that don't affect JS outside source text module record namespaces currently occur during source text module record Instantiate. I guess someone could completely run the phases of loading the dynamically imported module during HostImportModuleDynamically even up to calling Evaluate but that also seems surprising to me and might be problematic for things like Top Level Await that create asynchronous behavior during some phase of module loading. I'd also recommend not doing that / we could be more specific about this since features adding asynchrony would be in conflict with allowing that synchronous behavior I think.

[bmeck]

@weswigham I'm trying to understand this init step, don't have a clear picture of what that is. During the instantiate phase of the walk I would also be surprised by having side effects outside of your module record's namespace and only side effects like allocation of function declarations etc. that don't affect JS outside source text module record namespaces currently occur during source text module record Instantiate. I guess someone could completely run the phases of loading the dynamically imported module during HostImportModuleDynamically even up to calling Evaluate but that also seems surprising to me and might be problematic for things like Top Level Await that create asynchronous behavior during some phase of module loading. I'd also recommend not doing that / we could be more specific about this since features adding asynchrony would be in conflict with allowing that synchronous behavior I think.

[weswigham]

During the instantiate phase of the walk I would also be surprised by having side effects outside of your module record's namespace and only side effects like allocation of function declarations etc.

And that's true for an esm module in isolation - SourcetextModuleRecords are well-specified like that. Non-esm modules have no specified constraints that would lead you to believe that's the case, though, so it's not an invariant you can currently rely on.

but that also seems surprising to me and might be problematic for things like Top Level Await that create asynchronous behavior during some phase of module loading

Depends on which top-level await variant. The ones that don't actually "block" execution (so only bindings are immediately exported, but have no values until the async body manages to run, so the namespace is still immediately available) are (relatively) fine with it (it's just another point where execution order is nonobvious). That's theoretical, though (and is very difficult to discuss when it's a stage 1 proposal without a concrete shape or spec text) - the ability for a host to specify AbstractModuleRecords that seem to violate these invariants that SourceTextModuleRecord's loading process leads you to believe may exist is very possible (and may well actually make top level await difficult without caveats about non-sourcetext modules).

And as I keep saying, it might even be useful to enforce those invariants on other module kinds implemented in node (though doing so may be hard for some of them), but the spec doesn't seem to call for it right now (nor do I think it reasonably can) - that's presently a decision for node to make itself, as a host environment.

[weswigham]

During the instantiate phase of the walk I would also be surprised by having side effects outside of your module record's namespace and only side effects like allocation of function declarations etc.

And that's true for an esm module in isolation - SourcetextModuleRecords are well-specified like that. Non-esm modules have no specified constraints that would lead you to believe that's the case, though, so it's not an invariant you can currently rely on.

but that also seems surprising to me and might be problematic for things like Top Level Await that create asynchronous behavior during some phase of module loading

Depends on which top-level await variant. The ones that don't actually "block" execution (so only bindings are immediately exported, but have no values until the async body manages to run, so the namespace is still immediately available) are (relatively) fine with it (it's just another point where execution order is nonobvious). That's theoretical, though (and is very difficult to discuss when it's a stage 1 proposal without a concrete shape or spec text) - the ability for a host to specify AbstractModuleRecords that seem to violate these invariants that SourceTextModuleRecord's loading process leads you to believe may exist is very possible (and may well actually make top level await difficult without caveats about non-sourcetext modules).

And as I keep saying, it might even be useful to enforce those invariants on other module kinds implemented in node (though doing so may be hard for some of them), but the spec doesn't seem to call for it right now (nor do I think it reasonably can) - that's presently a decision for node to make itself, as a host environment.

[bmeck]

And as I keep saying, it might even be useful to enforce those invariants on other module kinds implemented in node (though doing so may be hard for some of them), but the spec doesn't seem to call for it right now (nor do I think it reasonably can) - that's presently a decision for node to make itself, as a host environment.

I discuss things both in regards to TC39 and Node. I do not think invariants on module records only apply to Node. This discussion increasingly makes it clear that very few guarantees are had currently, even if that is not the intention. We should continue discussing the intention of things at TC39 probably, regardless of what hosts choose to do in the mean time.

[bmeck]

And as I keep saying, it might even be useful to enforce those invariants on other module kinds implemented in node (though doing so may be hard for some of them), but the spec doesn't seem to call for it right now (nor do I think it reasonably can) - that's presently a decision for node to make itself, as a host environment.

I discuss things both in regards to TC39 and Node. I do not think invariants on module records only apply to Node. This discussion increasingly makes it clear that very few guarantees are had currently, even if that is not the intention. We should continue discussing the intention of things at TC39 probably, regardless of what hosts choose to do in the mean time.