-
Notifications
You must be signed in to change notification settings - Fork 30k
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
Allow nested data: imports with --experimental-network-imports #53998
Allow nested data: imports with --experimental-network-imports #53998
Conversation
Review requested:
|
See also: #53822 |
Co-authored-by: James M Snell <jasnell@gmail.com>
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.
TSC reached a consensus to remove this feature. So, there's no reason to land this PR anymore.
@RafaelGSS I disagree, I think it makes sense to land this to fix the behavior on release lines (also TSC consensus is not exactly relevant, it's up to Node.js collaborators, not the TSC, unless collaborators can't find consensus) |
I think in that case it should just be targeting release lines, instead of main? |
Also, it seems the diff here isn't working, there are multiple relevant test failures, maybe there's some git error (OP mentioned a test case in |
I tried checking out this branch and fixing the tests, unfortunately it's apparent that Thanks for sending this PR, and sorry it won't work out this time! |
This PR addresses issue [ISSUE_NUMBER] by allowing data: imports from other data: URLs when the --experimental-network-imports flag is used. This change modifies the condition in lib/internal/modules/esm/resolve.js to check for nested data: URLs correctly.
Additionally, a new test case has been added in
test/parallel/test-esm-data-url-import.js
to verify this behavior.Fixes: #53992