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

module: load source maps in commonjs translator #51033

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
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

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@GeoffreyBooth
Copy link
Member

@nodejs/loaders

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

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

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

@nodejs-github-bot
Copy link
Collaborator

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
53 checks passed
@nodejs-github-bot
Copy link
Collaborator

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 <duhamelantoine1995@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Dec 15, 2023
Merged
@privatenumber privatenumber mentioned this pull request Dec 19, 2023
Open
6 tasks
@aduh95 aduh95 added the lts-watch-v18.x PRs that may need to be released in v18.x. label Dec 24, 2023
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 <duhamelantoine1995@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@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. lts-watch-v18.x PRs that may need to be released in v18.x. 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