Skip to content
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

Source phase import tests importing from "<do not resolve>" are broken #4193

Open
woess opened this issue Aug 13, 2024 · 1 comment
Open

Comments

@woess
Copy link
Contributor

woess commented Aug 13, 2024

I believe the following source-phase-import tests are broken because they assume wrong/outdated evaluation order:

language/module-code/source-phase-import/import-source-binding-name.js
language/module-code/source-phase-import/import-source-binding-name2.js
language/module-code/source-phase-import/import-source-newlines.js

They all consist of import "../resources/ensure-linking-error_FIXTURE.js"
followed by import … from '<do not resolve>' and declare

negative:
  phase: resolution
  type: SyntaxError

assuming that ensure-linking-error_FIXTURE.js will cause a SyntaxError before attempting to load <do not resolve>.

For that to work, modules would have to be loaded during the linking (resolution) phase, which used to be the case in an older version of the spec, but is no longer the case in the latest spec since there's now a separate "loading" phase (LoadRequestedModules) that precedes linking, that will run into an error because the requested module cannot be found.

So I think these tests should probably be changed to point to an existing file, so that the loading phase succeeds and the linking phase throws the expected SyntaxError.

Apologies if I missed something (like <do not resolve> having a special meaning).

@ptomato
Copy link
Contributor

ptomato commented Aug 14, 2024

Seems related to #4124. From the conclusion there, those tests should be changed to point to an existing file, and if possible rewritten so that they pass instead of being negative.

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

No branches or pull requests

2 participants