-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
test: esm loader unknown builtin module #24183
Conversation
The test looks good to me, and we checked with @BridgeAR who believes it is a problem that he has seen and believes it is a bug in Node.js |
Tests are now passing after fixing the resolve hook following @devsnek advice (#24175 (comment)). |
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.
Thanks a lot for the awesome work!
@BridgeAR hello, do you know why the CI is failing at the linter phase? It is not indicating a linter error but a CI crash, imho. Thank you for your support. |
seems like the version of node on our ci doesn't support catch without a binding wrt Line 20 in 1f6c4ba
|
Looking forward to see this PR landed :) |
@franher - sure, let me run a full CI and then we should be good to go. |
landed as 0229e37 , thanks! |
PR-URL: #24183 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: #24183 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: nodejs#24183 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: #24183 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
Description
New test added as part of code-learn session. It should not land until #24175 is resolved.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes