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

module: load source maps in commonjs translator #51033

nodejs-github-bot merged 1 commit into from
Dec 13, 2023

Conversation

@privatenumber
Copy link
Contributor

@privatenumberprivatenumber 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-botnodejs-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
@privatenumberprivatenumber mentioned this pull request Dec 3, 2023
Closed
@privatenumberprivatenumber changed the title test: source map from cjs file in esm loader hookfix(hooks): source maps in cjs translatorDec 3, 2023
@privatenumberprivatenumber changed the title fix(hooks): source maps in cjs translatormodule: load source maps in commonjs translatorDec 3, 2023
@privatenumberprivatenumber marked this pull request as ready for review December 3, 2023 16:52
@privatenumberprivatenumberforce-pushed the 50649 branch 2 times, most recently from a3234b7 to b43f701CompareDecember 3, 2023 17:08
aduh95
aduh95 approved these changes Dec 3, 2023
View reviewed changes
Copy link
Contributor

@aduh95aduh95 left a comment

Choose a reason for hiding this comment

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

Thanks!

@aduh95aduh95 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-actionsgithub-actionsbot 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
@GeoffreyBoothGeoffreyBooth removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 5, 2023
@privatenumberprivatenumber 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

@GeoffreyBoothGeoffreyBooth 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!

@GeoffreyBoothGeoffreyBooth added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 5, 2023
@GeoffreyBoothGeoffreyBooth added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 5, 2023
@github-actionsgithub-actionsbot 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

@aduh95aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 9, 2023
@github-actionsgithub-actionsbot 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/

@debadree25debadree25 added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 13, 2023
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 13, 2023
@nodejs-github-botnodejs-github-bot merged commit 228bc5c into nodejs:mainDec 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]>
@RafaelGSSRafaelGSS mentioned this pull request Dec 15, 2023
Merged
@privatenumberprivatenumber 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]>
@richardlaurichardlau mentioned this pull request Mar 25, 2024
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnelljasnelljasnell approved these changes

@GeoffreyBoothGeoffreyBoothGeoffreyBooth approved these changes

@legendecaslegendecaslegendecas approved these changes

@aduh95aduh95aduh95 approved these changes

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.esmIssues and PRs related to the ECMAScript Modules implementation.needs-ciPRs that need a full CI run.source mapsIssues 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