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

RafaelGSS

LGTM!

lib/util.js

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

doc/api/util.md

nodejs-github-bot · 2024-11-03T10:40:05Z

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

nodejs-github-bot · 2024-11-03T14:42:23Z

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

nodejs-github-bot · 2024-11-03T15:52:38Z

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

marco-ippolito · 2024-11-03T17:20:02Z

Of course output matching on windows path is different, I have to update tests

nodejs-github-bot · 2024-11-04T08:59:56Z

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

doc/api/util.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

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 force-pushed the mapCallSite branch from a9977d8 to 20bd8b7 Compare November 2, 2024 15:44

marco-ippolito changed the title ~~util: add sourcemap support to getCallSite~~ util: add sourcemap support to getCallSites Nov 2, 2024

marco-ippolito force-pushed the mapCallSite branch from 20bd8b7 to 39af69d Compare November 2, 2024 15:46

RafaelGSS approved these changes Nov 2, 2024

View reviewed changes

anonrig reviewed Nov 2, 2024

View reviewed changes

lib/util.js Show resolved Hide resolved

lib/util.js Outdated Show resolved Hide resolved

lib/util.js Show resolved Hide resolved

test/fixtures/typescript/ts/test-get-callsite-explicit.ts Outdated Show resolved Hide resolved

anonrig reviewed Nov 2, 2024

View reviewed changes

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

marco-ippolito force-pushed the mapCallSite branch 2 times, most recently from dc30bee to c1d8caf Compare November 3, 2024 09:27

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

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

marco-ippolito force-pushed the mapCallSite branch from c1d8caf to 936828c Compare November 4, 2024 08:38

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

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

legendecas reviewed Nov 4, 2024

View reviewed changes

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

util: add sourcemap support to getCallSites

8a14517

marco-ippolito force-pushed the mapCallSite branch from 936828c to 8a14517 Compare November 4, 2024 11:41

marco-ippolito requested a review from legendecas November 4, 2024 11:42

legendecas approved these changes Nov 4, 2024

View reviewed changes

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

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

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

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

RafaelGSS left a comment

nodejs-github-bot commented Nov 3, 2024

nodejs-github-bot commented Nov 3, 2024

nodejs-github-bot commented Nov 3, 2024

marco-ippolito commented Nov 3, 2024 •

edited

Loading

nodejs-github-bot commented Nov 4, 2024

nodejs-github-bot commented Nov 4, 2024

nodejs-github-bot commented Nov 4, 2024

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

RafaelGSS left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Nov 3, 2024

nodejs-github-bot commented Nov 3, 2024

nodejs-github-bot commented Nov 3, 2024

marco-ippolito commented Nov 3, 2024 • edited Loading

nodejs-github-bot commented Nov 4, 2024

nodejs-github-bot commented Nov 4, 2024

nodejs-github-bot commented Nov 4, 2024

codecov bot commented Oct 30, 2024 •

edited

Loading

RafaelGSS left a comment •

edited

Loading

marco-ippolito commented Oct 31, 2024 •

edited

Loading

marco-ippolito commented Nov 3, 2024 •

edited

Loading