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

Update import defer tests for evaluation triggers #4341

Merged
merged 6 commits into from
Jan 24, 2025

Conversation

nicolo-ribaudo
Copy link
Member

Ref #4215.

This updates the tests that check what operations trigger evaluation of deferred modules, based on consensus in the last TC39 meeting. It's effectively what is included in tc39/proposal-defer-import-eval#53.

This PR doesn't include tests about what the results of those operations are (which will need to be tested for .then).

I suggest reviewing commit-by-commit:

  • first the first one
  • then the last one
  • then the second one
  • then the third one

All the tests are auto-generated from templates.

There is a bug with templates btw: their desc cannot start with [, because the generator will not escape/prefix it and so it becomes a Yaml array. I had to prefix all the descs with _ for this reason.

@nicolo-ribaudo nicolo-ribaudo requested a review from a team as a code owner December 12, 2024 15:37
@nicolo-ribaudo nicolo-ribaudo mentioned this pull request Dec 12, 2024
38 tasks
@nicolo-ribaudo nicolo-ribaudo changed the title Remove old tests about import defer evaluation triggers Update import defer tests for evaluation triggers Dec 12, 2024
@takikawa
Copy link
Contributor

I tried running this in my webkit prototype that's been updated to the new semantics, and as far as I can tell the test cases all run and agree with the implementation.

@nicolo-ribaudo nicolo-ribaudo requested a review from ptomato January 9, 2025 17:19
@nicolo-ribaudo nicolo-ribaudo linked an issue Jan 10, 2025 that may be closed by this pull request
38 tasks
@ptomato
Copy link
Contributor

ptomato commented Jan 20, 2025

There is a bug with templates btw: their desc cannot start with [, because the generator will not escape/prefix it and so it becomes a Yaml array. I had to prefix all the descs with _ for this reason.

I think this is probably working as intended — the .case frontmatter is supposed to be valid YAML, so it wouldn't be escaped, rather parsed and merged into the .template frontmatter. Would it work if you did something like desc: "[[DefineOwnProperty]]"?

@nicolo-ribaudo
Copy link
Member Author

No, because the generator parses that code, finds [[DefineOwnProperty]] as the value of desc, and then creates the generated files without the quotes around it.

@takikawa
Copy link
Contributor

@nicolo-ribaudo asked me to take another look at this to check the coverage, and so I looked through the test cases and as far as I can tell it covers the evaluation trigger cases for module namespaces in the spec well. So LGTM.

@Ms2ger
Copy link
Contributor

Ms2ger commented Jan 24, 2025

No, because the generator parses that code, finds [[DefineOwnProperty]] as the value of desc, and then creates the generated files without the quotes around it.

Please file an issue for this

@Ms2ger Ms2ger force-pushed the import-defer-updates branch from 9950d13 to 40dd470 Compare January 24, 2025 09:51
@Ms2ger Ms2ger merged commit e7a84b0 into tc39:main Jan 24, 2025
11 checks passed
@nicolo-ribaudo nicolo-ribaudo deleted the import-defer-updates branch January 24, 2025 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

import defer testing plan
4 participants