Skip to content

Conversation

@nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented May 22, 2025

Silly mistake by me in #3353, where I translated "[[AsyncEvaluation]] is false" to "[[AsyncEvaluationOrder]] is unset".

That's not true even in trivial cases. If you have module A that depends on module B, and B is async:

  • you first import A, which imports B. Once everything is done, B has [[AsyncEvaluationOrder]] set to done
  • then with a dynamic import you import B, whose re-Evaluation obviously completes synchronously (because it's already evaluated) but does not have [[AsyncEvaluationOrder]] set to unset.

This is different from the error case, where we can indeed assert that [[AsyncEvaluationOrder]] is unset. That's because in the error case we perform the assertion on what's in the stack, but we push modules to it only after checking that they are not already evaluated.

Copy link

@guybedford guybedford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. It might be slightly more revealing to say is ~unset~ or ~done~ here to help understand the cases directly instead of via negation.

@nicolo-ribaudo nicolo-ribaudo requested a review from bakkot June 27, 2025 08:52
@nicolo-ribaudo
Copy link
Member Author

[ready to merge]? :)

@linusg linusg added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Jul 3, 2025
@ljharb ljharb removed the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Jul 3, 2025
@ljharb
Copy link
Member

ljharb commented Jul 3, 2025

(that's only up to the editors)

@michaelficarra
Copy link
Member

And I'm not sure yet whether @syg will want to review this.

@nicolo-ribaudo
Copy link
Member Author

Ping @syg if you want to review this :)

@bakkot bakkot added the editor call to be discussed in the next editor call label Sep 22, 2025
@bakkot bakkot added ready to merge Editors believe this PR needs no further reviews, and is ready to land. and removed editor call to be discussed in the next editor call labels Sep 29, 2025
@bakkot
Copy link
Member

bakkot commented Sep 29, 2025

Ready to land but needs @jmdyck's suggestion pulled in when landing, which we can do.

@ljharb ljharb changed the title Adjust invalid assertion about [[AsyncEvaluationOrder]] Editorial: Adjust invalid assertion about [[AsyncEvaluationOrder]] Sep 30, 2025
@github-actions
Copy link

The rendered spec for this PR is available at https://tc39.es/ecma262/pr/3613.

@ljharb ljharb merged commit 55276b2 into tc39:main Sep 30, 2025
8 checks passed
ljharb added a commit that referenced this pull request Sep 30, 2025
@nicolo-ribaudo nicolo-ribaudo deleted the invalid-assertion branch October 3, 2025 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

editorial change ready to merge Editors believe this PR needs no further reviews, and is ready to land.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants