This repository has been archived by the owner on Aug 29, 2021. It is now read-only.
Normative: Avoid microtask queue delay when resolved #49
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Note, this is one of two possible solutions. I'm working on a writeup of how this would be solved instead through a microtask queue checkpoint. |
|
cc @GeorgNeis |
|
There is a possible tweak to this patch, where we only treat a module as synchronous if it completed synchronously the first time, and so any top-level await (even from a previous module graph traversal) will always take a turn through the job queue before the module runs. Maybe this would ameliorate the "Zalgo" concerns a little bit, but it would require extra bookkeeping. |
littledan
commented
Feb 22, 2019
guybedford
reviewed
Feb 25, 2019
spec.html
Outdated
| @@ -357,7 +357,7 @@ <h1>InnerModuleEvaluation( _module_, _stack_, _index_ )</h1> | |||
| 1. <ins>Let _evalCapability_ be ! NewPromiseCapability(%Promise%).</ins> | |||
| 1. <ins>Set _module_.[[ExecPromise]] to _evalCapability_.[[Promise]]. | |||
| 1. Set _index_ to _index_ + 1. | |||
| 1. Append _module_ to _stack_. | |||
| 1. Append _module_. | |||
There was a problem hiding this comment.
This seems like a typo?
0cdf693 to
ab2344e
Compare
|
Applied #49 (comment) in |
Closed
littledan
added a commit
to littledan/proposal-top-level-await
that referenced
this pull request
Mar 19, 2019
This patch is a variant on tc39#49 which determines which module subgraphs are to be executed synchronously based on syntax (whether the module contains a top-level await syntactically) and the dependency graph (whether it imports a module which contains a top-level await, recursively). This fixed check is designed to be more predictable and analyzable. Abstract module record changes: - The [[Async]] field stores whether this module or dependencies are async. It is expected to be initialized by the Linking phase. - The [[ExecutionPromise]] field stores the Promise related to the evaluation of a module whose [[Async]] field is *true*. - Evaluate() returns a Promise for [[Async]] modules, a completion record for sync modules which throw, or undefined otherwise. Cyclic Module Record changes: - A new [[ModuleAsync]] field stores whether this particular module is asynchronous (dependencies aside). - The ExecuteModule method on Cyclic Module Records takes an argument for the Promise capability, but only if that particular module is [[ModuleAsync]]. - The Link/Instantiate phase is used to propagate the [[Async]] field up the module graph to dependencies. - When there's a cycle, with some modules sync and some async, the whole cycle is considered async, with the Promise of each module set to the entrypoint of the cycle, although the cycle-closing edge will not actually be awaited (since this would be a deadlock). Source Text Module Record changes: - The check for whether a module contains a top-level await locally is in a ContainsAwait algorithm (TBD writing this out, but it should be static since await may not appear in a direct eval) - Module execution works as before if ContainsAwait is false, and works like an async function if ContainsAwait is true. Closes tc39#47, tc39#48, tc39#43
|
I would prefer to not take this approach, as it leaves whether a module is async or not up to whether it dynamically hits an await, as opposed to whether one is syntactically present (as in the current proposal). So, closing this PR. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #47, #48, #43