module: convert schema-only core module on `convertCJSFilenameToURL`
Fixes: https://github.com/nodejs/node/issues/58607
Review requested:
- [ ] @nodejs/loaders
From the issue description I don’t think its the validation that should be changed - it should be the URL that gets passed into the hooks that should be corrected (it should’ve been node:sea instead of sea, looks like somewhere in the CJS loader the conversion is missed since internally we use filenames and ids everywhere and only convert them to URLs when being passed into hooks).
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 90.10%. Comparing base (
faada65) to head (ee2dc06). Report is 12 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #58612 +/- ##
=======================================
Coverage 90.09% 90.10%
=======================================
Files 640 640
Lines 188271 188275 +4
Branches 36923 36923
=======================================
+ Hits 169625 169641 +16
+ Misses 11386 11344 -42
- Partials 7260 7290 +30
| Files with missing lines | Coverage Δ | |
|---|---|---|
| lib/internal/modules/customization_hooks.js | 100.00% <100.00%> (ø) |
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
From the issue description I don’t think its the validation that should be changed - it should be the URL that gets passed into the hooks that should be corrected (it should’ve been node:sea instead of sea, looks like somewhere in the CJS loader the conversion is missed since internally we use filenames and ids everywhere and only convert them to URLs when being passed into hooks).
Yeah, I read the code and think the issue should be in convertCJSFilenameToURL ? updated the code now
Failed to start CI
⚠ No approving reviews found ✘ Refusing to run CI on potentially unsafe PRhttps://github.com/nodejs/node/actions/runs/15542291850
https://github.com/nodejs/node/pull/58612#discussion_r2137646849 @himself65 in case you missed it
CI: https://ci.nodejs.org/job/node-test-pull-request/67545/
CI: https://ci.nodejs.org/job/node-test-pull-request/67634/
CI: https://ci.nodejs.org/job/node-test-pull-request/67635/
CI: https://ci.nodejs.org/job/node-test-pull-request/67636/
CI: https://ci.nodejs.org/job/node-test-pull-request/67639/
CI: https://ci.nodejs.org/job/node-test-pull-request/67648/
Landed in a705e240b197262d5a8eee39f11797d14cfc94cb