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

Open
awto opened this issue Dec 27, 2024 · 2 comments · May be fixed by #56402
Open

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

awto opened this issue Dec 27, 2024 · 2 comments · May be fixed by #56402

Comments

@awto
Copy link

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
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 linked a pull request Dec 29, 2024 that will close this issue
Open
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
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