proposal-compartments icon indicating copy to clipboard operation
proposal-compartments copied to clipboard

Support top-level-await

Open kriskowal opened this issue 5 years ago • 11 comments

As proposed at the time of writing, importNow returns an exotic module exports namespace object and throws an exception if the specified module or any of its transitive dependencies have not yet loaded. The import method returns a promise for exports because it may need to load the dependencies using the asynchronous loader hook #26.

Top-level-await introduces the possibility that importNow would need to do asynchronous to fully initialize the transitive dependencies. To that end, we have some options to bring the compartment proposal in alignment with top-level-await.

  1. importNow continues to work as-is for module graphs that lack any top-level-await.
    1. importNow returns a promise if any dependency uses top-level-await.
    2. importNow throws an exception if any dependency uses top-level-await.
  2. Remove importNow and rely on the import method to serve in its stead regardless of whether any dependency uses top-level-await.

The first option (and two suboptions) introduce a hazard in the face of changing dependencies. If any of a module’s transitive dependencies add a first top-level-await, the signature or behavior of importNow changes for that module in a way that breaks existing code. If we choose to keep importNow, we would need to socialize that adding a top-level-await to a published module is a breaking change and that it would require a major version number bump and a migration to be done safely.

Supporting top-level-await also requires ensuring that the compartment proposal meshes with the existing specified rules for initializing module subgraphs. I expect that this will require no adjustments to the interface as defined today.

Third-party module support #36 creates a case for keeping importNow, since third party module initializers can expect that their dependencies have already executed.

See #31 for a fresh attempt to capture this issue description, closed as duplicate.

kriskowal avatar Oct 22 '20 03:10 kriskowal

1.1. importNow throws an exception if any dependency uses top-level-await.

Is it possible to make those two APIs into one so the developer can decide if they want to throw when it containing a top-level-await?

For example:

const result = compartment.import(str)
if (result.type === 'async') {
    console.error('Promise:', result.promise)
    throw new Error('Cannot contain top-level-await')
} else {
    assert(result.type === 'sync')
    console.log(result.module)
}

Jack-Works avatar Oct 22 '20 03:10 Jack-Works

@jack-works That is certainly an option, and it suffers the same hazard. Code that failed to account for both cases would break. This would likely lead to a great deal of unnecessary ceremony and breakage.

kriskowal avatar Oct 22 '20 03:10 kriskowal

There was a significant amount of effort put into TLA to ensure it would not be breaking for a module's dependencies to use it. It would be unfortunate to expose it here.

devsnek avatar Oct 22 '20 03:10 devsnek

If we do not want to expose it, we need to disable sync import totally 👀

Jack-Works avatar Oct 22 '20 03:10 Jack-Works

Is it correct to say that application code can avoid being a victim of the breaking change by just always using import instead of importNow?

coder-mike avatar Oct 29 '20 00:10 coder-mike

Is it correct to say that application code can avoid being a victim of the breaking change by just always using import instead of importNow?

Yes. Regardless of these two options, import would always work.

kriskowal avatar Oct 29 '20 00:10 kriskowal

There was a significant amount of effort put into TLA to ensure it would not be breaking for a module's dependencies to use it.

@devsnek

How? As my last reading of TLA proposal, any module introducing TLA will be a breaking change and need to update its major semver.

hax avatar Oct 29 '20 05:10 hax

@hax that is incorrect. it creates a graph with async edges, from the perspective of esm code it doesn't change at all, and since import() already returns a promise, it isn't breaking.

devsnek avatar Oct 29 '20 12:10 devsnek

@devsnek what if code runs like this?

<script type="module">
import { setup } './something.js'
setup()
</script>
<script type="module">
assert(already_setup)
</script>

And one-day "something.js" is starting to use TLA?

Jack-Works avatar Oct 29 '20 14:10 Jack-Works

@Jack-Works it is unclear to me what that code does (is it using globals??), and I don't know the rules of evaluation for script type=module tags. but the above point relates to using await inside a module not being a breaking change within a single graph. your example uses two graphs which race each other, which seems dangerous even without using tla.

devsnek avatar Oct 29 '20 14:10 devsnek

your example uses two graphs which race each other, which seems dangerous even without using tla.

Scripts of <script type=module> (without async attr) are ensure to executed one by one.

Unfortunately TLA will break that. So introducing TLA in any modules which directly or indirectly referenced by the script files eventually become a breaking change and could cause random bugs which will be very hard to locate.

Of coz, someone argues that two <script type=module> should not share anything, but if that, people should use <script type=module async>, so this actually make the original design of <script type=module> meaningless.

hax avatar Jan 19 '21 10:01 hax