module: load source maps in commonjs translator

[privatenumber]

Addresses #51033 (comment)

PR waiting on #51033

[nodejs-github-bot]

Review requested:

• @nodejs/loaders

[GeoffreyBooth]

Why is this a new PR? Can you please just update the branch for #51033 with this code?

Then if you can refactor --experimental-loader into --import / register it can land.

[privatenumber]

I prefer to organize my PRs by the problem they address

To me, including this in #51033 doesn't make sense because the code changes in this PR are not necessary to fix the problem it address (#50839)

[GeoffreyBooth]

I understand that’s how you prefer to organize your PRs, but in this project we squash all commits for every PR, and we want each PR to be as contained as possible because we need to backport the squashed commits onto multiple release lines (21.x, 20.x, 18.x, etc.). In other words, we try to minimize the number of commits on main because each extra commit there is more work for the release team to prepare each release. Responding to code review notes is something that we expect people to do on the branch associated with the opened PR, not in new PRs.

[privatenumber]

Okay, in that case, I'll address your comments exclusively to the changes I made.

[GeoffreyBooth]

People often just push new commits to respond to each code review note. It doesn’t matter how many commits are on a branch because they all end up getting squashed anyway; but until the PR lands, if you ever need the history of how the PR looked at an earlier point, you still have it.

Thanks for working with us!

[privatenumber]

It was less about the commits (as I often squash/amend my commits in a PR anyway). More about the scope of the change and how easy it is for a 3rd person to understand relevant code changes.