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.

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

Source maps not working in CommonJS via Hooks API #50839

Closed
privatenumber opened this issue Nov 21, 2023 · 3 comments · Fixed by #51033
Closed

Source maps not working in CommonJS via Hooks API #50839

privatenumber opened this issue Nov 21, 2023 · 3 comments · Fixed by #51033

Comments

@privatenumber
Copy link
Contributor

privatenumber commented Nov 21, 2023

Version

v21.2.0

Platform

All

Subsystem

All

What steps will reproduce the bug?

Reproduction: https://github.com/privatenumber/bug-node-hooks-cjs-sourcemap

I added a Hook (load) that just reads the CommonJS file and returns the file as is (the same example from the docs). When loading a file with an inline source map, the error stack does not show the correct line number. You can see this in the reproduction by running npm start.

In contrast, it works when the same file is directly with --enable-source-maps without registering a Hook. You can see this in the reproduction by running npm run works.

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

Every time when CJS is loaded with the Hooks API.

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

For the source map to work

What do you see instead?

Incorrect error stack line numbers

Additional information

CJS support in Hooks were added in #47999
No response

@aduh95
Copy link
Contributor

aduh95 commented Nov 22, 2023

Thanks for the report, would you like to contribute a fix? If not, it would go a long way if you could write a test that covers that use-case.

@privatenumber
Copy link
Contributor Author

For now, I opened a PR with a failing test #51033

Will try to look into a fix

By the way, is there a way to run only the test-esm-loader-hooks.mjs test?
A little slow running everything with make -j4 jstest.

@aduh95
Copy link
Contributor

aduh95 commented Dec 3, 2023

is there a way to run only the test-esm-loader-hooks.mjs test?

tools/test.py test/es-module/test-esm-loader-hooks.mjs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants