-
Notifications
You must be signed in to change notification settings - Fork 29.1k
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: fix support for URL
instances in import.meta.resolve
#54690
Conversation
Review requested:
|
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.
Is it what browsers do too?
Trying to run |
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.
For context: I encountered an issue where import.meta.resolve(path, baseAsURLInstance)
b0rks the entire execution when a loader is added (it ultimately throws a structured clone error, which gets swallowed).
This pull request is an amazing example of collaboration. Thank you all. |
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.
LGTM.
Does this also require adjustments to typings in JSDoc (
node/lib/internal/modules/esm/loader.js
Line 507 in 05c048d
* @param {string} originalSpecifier The specified URL path of the module to |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #54690 +/- ##
==========================================
- Coverage 87.61% 87.60% -0.02%
==========================================
Files 650 650
Lines 182834 182836 +2
Branches 35382 35385 +3
==========================================
- Hits 160194 160177 -17
- Misses 15925 15926 +1
- Partials 6715 6733 +18
|
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
PR is currently blocked from landing due to unreliable CI |
This comment was marked as outdated.
This comment was marked as outdated.
Landed in 2ef33af |
PR-URL: #54690 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: LiviaMedeiros <livia@cirno.name> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #54690 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: LiviaMedeiros <livia@cirno.name> Reviewed-By: James M Snell <jasnell@gmail.com>
No description provided.