-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
esm: increase test coverage of edge cases #47033
Conversation
Failed to start CI- Validating Jenkins credentials ✔ Jenkins credentials valid - Starting PR CI job ✘ Failed to start PR CI: 403 Forbiddenhttps://github.com/nodejs/node/actions/runs/4385304820 |
What does this mean? This PR is testing that |
Correct. This should help loader authors who are reviewing the PR understand what has changed with the off-thread PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙌
Landed in 841f6b3 |
Hi, @aduh95. Could you please create a manual backport for v19.x? It didn't land cleanly because the suite on which the tests were included is the same one that contains: it('should work without worker permission', async () => { which wasn't landed in v19.x. |
PR-URL: nodejs#47033 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Jacob Smith <jacob@frende.me>
PR-URL: nodejs/node#47033 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Jacob Smith <jacob@frende.me>
This PR adds some important assertions in preparation for the off-thread PR.
require('node:worker_thread').workerData
.import.meta.resolve
is currently available from loader modules (that won't be the case when it's taken off thread).import.meta.resolve
using a specifier that returns a never-settling promise exits the process with code 13. (not sure about this one, I think it's better than leaving the process in a dead lock forever, but also code 13 won't make as much sense if there's no longer a top-level await).//cc @nodejs/loaders