Skip to content
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.

Sign up for GitHub

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

Jump to bottom

CJS nextLoader callback in loader hook doesn't use changed URL #56376

Closed
awto opened this issue Dec 27, 2024 · 2 comments
Closed

CJS nextLoader callback in loader hook doesn't use changed URL #56376

awto opened this issue Dec 27, 2024 · 2 comments

Comments

@awto
Copy link
Contributor

awto commented Dec 27, 2024

Version

v23.5.0

Platform

Darwin MBP 23.6.0 Darwin Kernel Version 23.6.0: Mon Jul 29 21:13:04 PDT 2024; root:xnu-10063.141.2~1/RELEASE_ARM64_T6020 arm64

Subsystem

loader

What steps will reproduce the bug?

I believe there is a typo at node/lib/internal/modules/cjs
/loader.js:1147.

It doesn't use urlFromHook but only the original url. So, the hook cannot change the URL for the next stage in the chain.

How often does it reproduce? Is there a required condition?

always

What is the expected behavior? Why is that the expected behavior?

Since there is a parameter for passing URL to nextLoad it should be used.

What do you see instead?

The parameter passed to nextLoad is ignored.

Additional information

No response
@marco-ippolito
Copy link
Member

Would you like to send a PR?

@awto
Copy link
Contributor Author

awto commented Dec 29, 2024

Yes, I'll do, thanks

awto pushed a commit to awto/node that referenced this issue Dec 29, 2024
@vitaliilbr
module: fixing url change in load sync hook chain
3bbcf64 
Fixes: nodejs#56376
@awto awto mentioned this issue Dec 29, 2024
Closed
awto added a commit to awto/node that referenced this issue Dec 29, 2024
@awto
module: fixing url change in load sync hook chain
4b2b413 
Fixes: nodejs#56376
awto added a commit to awto/node that referenced this issue Jan 16, 2025
@awto
module: fixing url change in load sync hook chain
084e18a 
Fixes: nodejs#56376
aduh95 pushed a commit that referenced this issue Jan 27, 2025
@awto @aduh95
module: fixing url change in load sync hook chain
f4ce91b 
Fixes: #56376
PR-URL: #56402
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
aduh95 pushed a commit that referenced this issue Jan 30, 2025
@awto @aduh95
module: fixing url change in load sync hook chain
7347d34 
Fixes: #56376
PR-URL: #56402
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

module: fixing url change in load sync hook chain awto/node
2 participants
@awto @marco-ippolito