GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — module: load source maps in `commonjs` translator by privatenumber · Pull Request #51033 · nodejs/node

Conversation

@privatenumber

@privatenumber privatenumber commented Dec 3, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

fixes #50839

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss and remorses reacted with rocket emoji
All reactions
  • 🚀 2 reactions
@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 Node.js core tests and test infrastructure. labels Dec 3, 2023
@privatenumber privatenumber mentioned this pull request Dec 3, 2023
Source maps not working in CommonJS via Hooks API #50839
Closed
@privatenumber
privatenumber force-pushed the 50649 branch from 6372646 to 3481c31 Compare December 3, 2023 16:48
@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 force-pushed the 50649 branch from 3481c31 to ba29aac Compare December 3, 2023 16:51
@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

@aduh95 aduh95 left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
privatenumber reacted with heart emoji
All reactions
  • ❤️ 1 reaction
@aduh95 aduh95 added author ready PRs with CI started, the required approvals, and no outstanding review comments. source maps Issues and PRs related to source map support. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. and removed test Issues and PRs related to Node.js core tests and test infrastructure. labels Dec 3, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Dec 3, 2023
@nodejs-github-bot

nodejs-github-bot commented Dec 3, 2023

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Dec 4, 2023

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Dec 4, 2023

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@GeoffreyBooth

GeoffreyBooth commented Dec 4, 2023

Copy link
Copy Markdown
Member

@nodejs/loaders
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
GeoffreyBooth
GeoffreyBooth reviewed Dec 4, 2023
View reviewed changes
Comment thread test/es-module/test-esm-loader-hooks.mjs Resolved

Uh oh!

There was an error while loading. Please reload this page.
GeoffreyBooth
GeoffreyBooth reviewed Dec 5, 2023
View reviewed changes
Comment thread test/es-module/test-esm-loader-hooks.mjs Resolved

Uh oh!

There was an error while loading. Please reload this page.
@GeoffreyBooth GeoffreyBooth removed the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Dec 5, 2023
@privatenumber privatenumber mentioned this pull request Dec 5, 2023
module: load source maps in commonjs translator #51053
Closed
@privatenumber
privatenumber force-pushed the 50649 branch from 4823151 to 8ccde54 Compare December 5, 2023 07:01
aduh95
aduh95 reviewed Dec 5, 2023
View reviewed changes
Comment thread test/es-module/test-esm-loader-hooks.mjs Resolved

Uh oh!

There was an error while loading. Please reload this page.
aduh95
aduh95 approved these changes Dec 5, 2023
View reviewed changes
GeoffreyBooth
GeoffreyBooth approved these changes Dec 5, 2023
View reviewed changes

@GeoffreyBooth GeoffreyBooth left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thank you!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@GeoffreyBooth GeoffreyBooth added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Dec 5, 2023
@GeoffreyBooth GeoffreyBooth added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Dec 5, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Dec 5, 2023
@nodejs-github-bot

nodejs-github-bot commented Dec 5, 2023

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
legendecas
legendecas approved these changes Dec 5, 2023
View reviewed changes
@aduh95

aduh95 commented Dec 5, 2023

Copy link
Copy Markdown
Contributor

@privatenumber can you please rebase all your commits on top of main? Merge commits tend to break our tooling
privatenumber reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@privatenumber
module: load source maps in commonjs translator
2f70bf3
@privatenumber
privatenumber force-pushed the 50649 branch from 13ceea2 to 2f70bf3 Compare December 6, 2023 06:39
jasnell
jasnell approved these changes Dec 9, 2023
View reviewed changes
@aduh95 aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Dec 9, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Dec 9, 2023
@nodejs-github-bot

nodejs-github-bot commented Dec 9, 2023

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Dec 10, 2023

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@debadree25 debadree25 added the commit-queue PRs queued for automated landing through the Commit Queue. label Dec 13, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Dec 13, 2023
@nodejs-github-bot
nodejs-github-bot merged commit 228bc5c into nodejs:main Dec 13, 2023
@nodejs-github-bot

nodejs-github-bot commented Dec 13, 2023

Copy link
Copy Markdown
Collaborator

Landed in 228bc5c
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
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
v21.5.0 proposal #51166
Merged
@privatenumber privatenumber mentioned this pull request Dec 19, 2023
Surprising module resolution behavior vs. esbuild and bun privatenumber/tsx#442
Open
4 of 6 tasks
@aduh95 aduh95 added the lts-watch-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 <[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
v20.12.0 proposal #52212
Merged
@aduh95 aduh95 removed the lts-watch-v18.x label Nov 2, 2024
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

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. 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

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@privatenumber @nodejs-github-bot @GeoffreyBooth @aduh95 @jasnell @legendecas @debadree25
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.