util: add sourcemap support to getCallSites #55589

marco-ippolito · 2024-10-29T14:43:42Z

Fixes: #55109

nodejs-github-bot · 2024-10-29T14:43:47Z

Review requested:

@nodejs/loaders
@nodejs/typescript

codecov · 2024-10-30T13:11:01Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.40%. Comparing base (68dc15e) to head (8a14517).
Report is 20 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #55589      +/-   ##
==========================================
- Coverage   88.40%   88.40%   -0.01%     
==========================================
  Files         654      654              
  Lines      187637   187640       +3     
  Branches    36098    36103       +5     
==========================================
- Hits       165887   165879       -8     
- Misses      14994    15002       +8     
- Partials     6756     6759       +3

Files with missing lines	Coverage Δ
lib/util.js	`97.75% <100.00%> (+0.43%)`	⬆️

... and 33 files with indirect coverage changes

doc/api/module.md

lib/internal/source_map/source_map_cache.js

test/fixtures/typescript/ts/test-callsite.ts

doc/api/module.md

lib/internal/source_map/source_map_cache.js

test/es-module/test-module-get-callsite.mjs

RafaelGSS

I agree that if we want to fix #55109 instead of adding a new function, we should just improve the support of the existing util.getCallSite(). What about accepting a config and then proceeding with module mapping inside node:util?

marco-ippolito · 2024-10-31T10:25:44Z

I agree that if we want to fix #55109 instead of adding a new function, we should just improve the support of the existing util.getCallSite(). What about accepting a config and then proceeding with module mapping inside node:util?

With a new function it is more flexibile because you can decide which frame to map, instead of mapping all of them, and you can also mock.
It doesn't exclude that we could also improve support in util.getCallSite by adding an option.

test/es-module/test-module-map-callsite.mjs

legendecas · 2024-10-31T14:29:42Z

Not a big fan of the polymorphism that derived from the ambiguity of the name util.getCallSite which actually returns an array of call site object.

Submitted #55626 to not hijacking this thread.

doc/api/module.md

nodejs-github-bot · 2024-11-04T12:32:51Z

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

nodejs-github-bot · 2024-11-04T16:06:49Z

Landed in d35cde6

PR-URL: #55589 Fixes: #55109 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

Notable changes: doc: * enforce strict policy to semver-major releases (Rafael Gonzaga) #55732 src: * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) #55697 util: * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) #55589 * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) #55626 PR-URL: TODO

Notable changes: doc: * enforce strict policy to semver-major releases (Rafael Gonzaga) #55732 src: * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) #55697 util: * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) #55589 * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) #55626 PR-URL: #55921

Notable changes: doc: * enforce strict policy to semver-major releases (Rafael Gonzaga) nodejs#55732 src: * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) nodejs#55697 util: * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) nodejs#55589 * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) nodejs#55626 PR-URL: nodejs#1

Notable changes: doc: * enforce strict policy to semver-major releases (Rafael Gonzaga) nodejs#55732 src: * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) nodejs#55697 util: * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) nodejs#55589 * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) nodejs#55626 PR-URL: https://github.com/nodejs/aduh95/pull/1

Notable changes: doc: * enforce strict policy to semver-major releases (Rafael Gonzaga) nodejs#55732 src: * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) nodejs#55697 util: * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) nodejs#55589 * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) nodejs#55626 PR-URL: #1

Notable changes: doc: * enforce strict policy to semver-major releases (Rafael Gonzaga) #55732 src: * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) #55697 util: * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) #55589 * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) #55626 PR-URL: #55921

PR-URL: nodejs#55589 Fixes: nodejs#55109 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>

Notable changes: doc: * enforce strict policy to semver-major releases (Rafael Gonzaga) nodejs#55732 src: * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) nodejs#55697 util: * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) nodejs#55589 * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) nodejs#55626 PR-URL: nodejs#55921

Notable changes: doc: * enforce strict policy to semver-major releases (Rafael Gonzaga) nodejs#55732 sqlite: * (SEMVER-MINOR) add support for SQLite Session Extension (Bart Louwers) nodejs#54181 src: * (SEMVER-MINOR) add cli option to preserve env vars on dr (Rafael Gonzaga) nodejs#55697 util: * (SEMVER-MINOR) add sourcemap support to getCallSites (Marco Ippolito) nodejs#55589 * (SEMVER-MINOR) fix util.getCallSites plurality (Chengzhong Wu) nodejs#55626 PR-URL: TODO

nodejs-github-bot added module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run. labels Oct 29, 2024

marco-ippolito force-pushed the mapCallSite branch from 9e2675f to b31da8d Compare October 29, 2024 14:44

marco-ippolito marked this pull request as ready for review October 30, 2024 10:54

marco-ippolito force-pushed the mapCallSite branch from b31da8d to 9777431 Compare October 30, 2024 12:00

marco-ippolito added the semver-minor PRs that contain new features and should be released in the next minor version. label Oct 30, 2024

anonrig reviewed Oct 30, 2024

View reviewed changes

marco-ippolito force-pushed the mapCallSite branch from 9777431 to 13ad74f Compare October 30, 2024 15:11

joyeecheung reviewed Oct 30, 2024

View reviewed changes

marco-ippolito force-pushed the mapCallSite branch 5 times, most recently from 2bc03cf to fe8f181 Compare October 30, 2024 19:09

marco-ippolito added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 31, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 31, 2024

This comment was marked as outdated.

Sign in to view

RafaelGSS reviewed Oct 31, 2024

View reviewed changes

This comment was marked as outdated.

Sign in to view

This comment has been minimized.

Sign in to view

marco-ippolito commented Oct 31, 2024

View reviewed changes

test/es-module/test-module-map-callsite.mjs Outdated Show resolved Hide resolved

RafaelGSS reviewed Oct 31, 2024

View reviewed changes

doc/api/module.md Outdated Show resolved Hide resolved

marco-ippolito force-pushed the mapCallSite branch 2 times, most recently from 507688e to c1bc9e4 Compare November 2, 2024 11:23

marco-ippolito marked this pull request as draft November 2, 2024 11:36

marco-ippolito force-pushed the mapCallSite branch from c1bc9e4 to a9977d8 Compare November 2, 2024 11:49

marco-ippolito added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 4, 2024

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 4, 2024

nodejs-github-bot merged commit d35cde6 into nodejs:main Nov 4, 2024
59 checks passed

RafaelGSS mentioned this pull request Nov 19, 2024

2024-11-20, Version 23.3.0 (Current), @RafaelGSS #55921

Merged

This comment was marked as spam.

Sign in to view

RafaelGSS mentioned this pull request Nov 22, 2024

2024-11-29, Version 23.3.1 (Current), @RafaelGSS RafaelGSS/node#8

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

util: add sourcemap support to getCallSites #55589

util: add sourcemap support to getCallSites #55589

marco-ippolito commented Oct 29, 2024

nodejs-github-bot commented Oct 29, 2024

codecov bot commented Oct 30, 2024 •

edited

Loading

This comment was marked as outdated.

RafaelGSS left a comment •

edited

Loading

marco-ippolito commented Oct 31, 2024 •

edited

Loading

This comment was marked as outdated.

This comment has been minimized.

legendecas commented Oct 31, 2024

nodejs-github-bot commented Nov 4, 2024

nodejs-github-bot commented Nov 4, 2024

This comment was marked as spam.

util: add sourcemap support to getCallSites #55589

util: add sourcemap support to getCallSites #55589

Conversation

marco-ippolito commented Oct 29, 2024

nodejs-github-bot commented Oct 29, 2024

codecov bot commented Oct 30, 2024 • edited Loading

Codecov Report

This comment was marked as outdated.

RafaelGSS left a comment • edited Loading

Choose a reason for hiding this comment

marco-ippolito commented Oct 31, 2024 • edited Loading

This comment was marked as outdated.

This comment has been minimized.

legendecas commented Oct 31, 2024

nodejs-github-bot commented Nov 4, 2024

nodejs-github-bot commented Nov 4, 2024

This comment was marked as spam.

codecov bot commented Oct 30, 2024 •

edited

Loading

RafaelGSS left a comment •

edited

Loading

marco-ippolito commented Oct 31, 2024 •

edited

Loading