Skip to content

module: load source maps in commonjs translator #51033

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
Merged
merged 1 commit into from
Dec 13, 2023
Merged

module: load source maps in commonjs translator #51033

merged 1 commit into from
Dec 13, 2023

Conversation

@privatenumber
Copy link
Contributor

@privatenumber privatenumber commented Dec 3, 2023 •
edited
Loading

fixes #50839

Development command (for future reference):
make -s build-addons && tools/test.py test/es-module/test-esm-loader-hooks.mjs

@nodejs-github-bot nodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Dec 3, 2023
@privatenumber privatenumber mentioned this pull request Dec 3, 2023
Closed
@privatenumber privatenumber changed the title test: source map from cjs file in esm loader hook fix(hooks): source maps in cjs translator Dec 3, 2023
@privatenumber privatenumber changed the title fix(hooks): source maps in cjs translator module: load source maps in commonjs translator Dec 3, 2023
@privatenumber privatenumber marked this pull request as ready for review December 3, 2023 16:52
@privatenumber privatenumber force-pushed the 50649 branch 2 times, most recently from a3234b7 to b43f701 Compare December 3, 2023 17:08
aduh95
aduh95 approved these changes Dec 3, 2023
View reviewed changes
Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. source maps Issues and PRs related to source map support. request-ci Add this label to start a Jenkins CI on a PR. and removed test Issues and PRs related to the tests. labels Dec 3, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 3, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 3, 2023

CI: https://ci.nodejs.org/job/node-test-pull-request/56062/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 4, 2023

CI: https://ci.nodejs.org/job/node-test-pull-request/56069/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 4, 2023

CI: https://ci.nodejs.org/job/node-test-pull-request/56078/

@GeoffreyBooth
Copy link
Member

GeoffreyBooth commented Dec 4, 2023

@nodejs/loaders

GeoffreyBooth
GeoffreyBooth reviewed Dec 4, 2023
View reviewed changes
GeoffreyBooth
GeoffreyBooth reviewed Dec 5, 2023
View reviewed changes
@GeoffreyBooth GeoffreyBooth removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 5, 2023
@privatenumber privatenumber mentioned this pull request Dec 5, 2023
Closed
aduh95
aduh95 reviewed Dec 5, 2023
View reviewed changes
GeoffreyBooth
GeoffreyBooth approved these changes Dec 5, 2023
View reviewed changes
Copy link
Member

@GeoffreyBooth GeoffreyBooth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@GeoffreyBooth GeoffreyBooth added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 5, 2023
@GeoffreyBooth GeoffreyBooth added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 5, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 5, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 5, 2023

CI: https://ci.nodejs.org/job/node-test-pull-request/56102/

@aduh95
Copy link
Contributor

aduh95 commented Dec 5, 2023

@privatenumber can you please rebase all your commits on top of main? Merge commits tend to break our tooling

@aduh95 aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 9, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 9, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 9, 2023

CI: https://ci.nodejs.org/job/node-test-pull-request/56187/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 10, 2023

CI: https://ci.nodejs.org/job/node-test-pull-request/56196/

@debadree25 debadree25 added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 13, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 13, 2023
@nodejs-github-bot nodejs-github-bot merged commit 228bc5c into nodejs:main Dec 13, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Dec 13, 2023

Landed in 228bc5c

RafaelGSS pushed a commit that referenced this pull request Dec 15, 2023
@privatenumber @RafaelGSS
module: load source maps in commonjs translator
aa2be4b 
PR-URL: #51033
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Dec 15, 2023
Merged
@privatenumber privatenumber mentioned this pull request Dec 19, 2023
Open
6 tasks
richardlau pushed a commit that referenced this pull request Mar 25, 2024
@privatenumber @richardlau
module: load source maps in commonjs translator
5fc10ca 
PR-URL: #51033
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@richardlau richardlau mentioned this pull request Mar 25, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@GeoffreyBooth GeoffreyBooth GeoffreyBooth approved these changes

@legendecas legendecas legendecas approved these changes

@aduh95 aduh95 aduh95 approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. source maps Issues and PRs related to source map support.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Source maps not working in CommonJS via Hooks API

7 participants

@privatenumber @nodejs-github-bot @GeoffreyBooth @aduh95 @jasnell @legendecas @debadree25