Skip to content

Adjust invalid assertion about [[AsyncEvaluationOrder]] #3613

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants