lib,src: isInsideNodeModules should test on the first non-internal frame #60991

legendecas · 2025-12-08T12:55:51Z

Avoid checking up to 100 frames every time on functions like require('node:url').parse, when the user application never invokes them with --disable-deprecation=DEP0169 is present (so urlParseWarned will never be true).

Before #55286, isInsideNodeModules only checks the topmost non-internal frame. This restores the original behavior.

node/lib/url.js

Line 135 in 6e474c0

if (!urlParseWarned && !isInsideNodeModules(100, true)) {

nodejs-github-bot · 2025-12-08T12:55:56Z

Review requested:

@nodejs/loaders
@nodejs/performance
@nodejs/url

joyeecheung · 2025-12-08T13:40:42Z

Not for this PR, though it seems useful to have a helper specifically for deprecations and other one-off warnings that skips the check if it's disabled/already emitted/checked enough times.

codecov · 2025-12-08T14:38:25Z

Codecov Report

❌ Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.52%. Comparing base (6e474c0) to head (780517c).
⚠️ Report is 14 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_util.cc	71.42%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #60991      +/-   ##
==========================================
- Coverage   88.52%   88.52%   -0.01%     
==========================================
  Files         703      703              
  Lines      208430   208424       -6     
  Branches    40206    40197       -9     
==========================================
- Hits       184513   184501      -12     
+ Misses      15947    15937      -10     
- Partials     7970     7986      +16

Files with missing lines	Coverage Δ
lib/buffer.js	`100.00% <100.00%> (ø)`
lib/internal/modules/cjs/loader.js	`97.97% <100.00%> (-0.05%)`	⬇️
lib/punycode.js	`94.72% <100.00%> (ø)`
lib/url.js	`100.00% <100.00%> (ø)`
src/node_util.cc	`80.95% <71.42%> (+0.95%)`	⬆️

... and 39 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2025-12-08T16:53:55Z

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

anonrig · 2025-12-08T18:10:09Z

src/node_util.cc

-      result = true;
-      break;
-    }
+    result = script_name_str.find("/node_modules/") != std::string::npos ||


We can just use std::string::contains here I believe

It's C++23 for the string classes.

nodejs-github-bot · 2025-12-08T20:51:07Z

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

nodejs-github-bot · 2025-12-10T13:16:53Z

Commit Queue failed

- Loading data for nodejs/node/pull/60991 ✔ Done loading data for nodejs/node/pull/60991 ----------------------------------- PR info ------------------------------------ Title lib,src: isInsideNodeModules should test on the first non-internal frame (#60991) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch legendecas:benchmark-isinsidenodemodules -> nodejs:main Labels c++, lib / src, author ready, needs-ci, lts-watch-v20.x, lts-watch-v22.x Commits 2 - benchmark: add microbench on isInsideNodeModules - lib,src: isInsideNodeModules should test on the first non-internal frame Committers 1 - Chengzhong Wu <legendecas@gmail.com> PR-URL: https://github.com/nodejs/node/pull/60991 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/60991 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> -------------------------------------------------------------------------------- ℹ This PR was created on Mon, 08 Dec 2025 12:55:51 GMT ✔ Approvals: 4 ✔ - Joyee Cheung (@joyeecheung) (TSC): https://github.com/nodejs/node/pull/60991#pullrequestreview-3552088477 ✔ - Marco Ippolito (@marco-ippolito) (TSC): https://github.com/nodejs/node/pull/60991#pullrequestreview-3552116038 ✔ - Daniel Lemire (@lemire): https://github.com/nodejs/node/pull/60991#pullrequestreview-3552297116 ✔ - Colin Ihrig (@cjihrig): https://github.com/nodejs/node/pull/60991#pullrequestreview-3552456556 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2025-12-08T20:51:07Z: https://ci.nodejs.org/job/node-test-pull-request/70440/ - Querying data for job/node-test-pull-request/70440/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 60991 From https://github.com/nodejs/node * branch refs/pull/60991/merge -> FETCH_HEAD ✔ Fetched commits as 83ba6b107a06..780517cbdf8d -------------------------------------------------------------------------------- [main 7e6314c3be] benchmark: add microbench on isInsideNodeModules Author: Chengzhong Wu <legendecas@gmail.com> Date: Mon Dec 8 12:19:55 2025 +0000 1 file changed, 52 insertions(+) create mode 100644 benchmark/internal/util_isinsidenodemodules.js Auto-merging lib/internal/modules/cjs/loader.js [main cebd76f992] lib,src: isInsideNodeModules should test on the first non-internal frame Author: Chengzhong Wu <legendecas@gmail.com> Date: Mon Dec 8 12:36:38 2025 +0000 8 files changed, 58 insertions(+), 29 deletions(-) create mode 100644 test/parallel/test-internal-util-isinsidenodemodules.js ✔ Patches applied There are 2 commits in the PR. Attempting autorebase. (node:2398) [DEP0190] DeprecationWarning: Passing args to a child process with shell option true can lead to security vulnerabilities, as the arguments are not escaped, only concatenated. (Use `node --trace-deprecation ...` to show where the warning was created) Rebasing (2/4) Executing: git node land --amend --yes --------------------------------- New Message ---------------------------------- benchmark: add microbench on isInsideNodeModules

PR-URL: #60991
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Daniel Lemire <daniel@lemire.me>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

[detached HEAD 1ceb3af622] benchmark: add microbench on isInsideNodeModules
Author: Chengzhong Wu <legendecas@gmail.com>
Date: Mon Dec 8 12:19:55 2025 +0000
1 file changed, 52 insertions(+)
create mode 100644 benchmark/internal/util_isinsidenodemodules.js
Rebasing (3/4)
Rebasing (4/4)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
lib,src: isInsideNodeModules should test on the first non-internal frame

PR-URL: #60991
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Daniel Lemire <daniel@lemire.me>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

[detached HEAD ae6a956b81] lib,src: isInsideNodeModules should test on the first non-internal frame
Author: Chengzhong Wu <legendecas@gmail.com>
Date: Mon Dec 8 12:36:38 2025 +0000
8 files changed, 58 insertions(+), 29 deletions(-)
create mode 100644 test/parallel/test-internal-util-isinsidenodemodules.js
Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/20099802384

PR-URL: #60991 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

nodejs-github-bot · 2025-12-10T13:59:38Z

Landed in 83ba6b1...32ea48d

PR-URL: #60991 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

benchmark: add microbench on isInsideNodeModules

c2e83bf

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Dec 8, 2025

lib,src: isInsideNodeModules should test on the first non-internal frame

780517c

legendecas force-pushed the benchmark-isinsidenodemodules branch from 16df4ce to 780517c Compare December 8, 2025 13:21

joyeecheung approved these changes Dec 8, 2025

View reviewed changes

marco-ippolito approved these changes Dec 8, 2025

View reviewed changes

lemire approved these changes Dec 8, 2025

View reviewed changes

cjihrig approved these changes Dec 8, 2025

View reviewed changes

legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 8, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 8, 2025

anonrig reviewed Dec 8, 2025

View reviewed changes

aduh95 added lts-watch-v20.x PRs that may need to be released in v20.x lts-watch-v22.x PRs that may need to be released in v22.x labels Dec 8, 2025

aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Dec 8, 2025

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Dec 10, 2025

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 10, 2025

nodejs-github-bot closed this Dec 10, 2025

legendecas deleted the benchmark-isinsidenodemodules branch December 10, 2025 15:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

lib,src: isInsideNodeModules should test on the first non-internal frame #60991

lib,src: isInsideNodeModules should test on the first non-internal frame #60991

Uh oh!

legendecas commented Dec 8, 2025 •

edited

Loading

Uh oh!

nodejs-github-bot commented Dec 8, 2025

Uh oh!

joyeecheung commented Dec 8, 2025 •

edited

Loading

Uh oh!

codecov bot commented Dec 8, 2025 •

edited

Loading

Uh oh!

nodejs-github-bot commented Dec 8, 2025

Uh oh!

anonrig Dec 8, 2025

Uh oh!

Renegade334 Dec 8, 2025

Uh oh!

nodejs-github-bot commented Dec 8, 2025

Uh oh!

nodejs-github-bot commented Dec 10, 2025

PR-URL: #60991
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Daniel Lemire <daniel@lemire.me>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

PR-URL: #60991
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Daniel Lemire <daniel@lemire.me>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Uh oh!

nodejs-github-bot commented Dec 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Uh oh!

lib,src: isInsideNodeModules should test on the first non-internal frame #60991

lib,src: isInsideNodeModules should test on the first non-internal frame #60991

Uh oh!

Conversation

legendecas commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Dec 8, 2025

Uh oh!

joyeecheung commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nodejs-github-bot commented Dec 8, 2025

Uh oh!

anonrig Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Renegade334 Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Dec 8, 2025

Uh oh!

nodejs-github-bot commented Dec 10, 2025

PR-URL: #60991 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

PR-URL: #60991 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Uh oh!

nodejs-github-bot commented Dec 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

legendecas commented Dec 8, 2025 •

edited

Loading

joyeecheung commented Dec 8, 2025 •

edited

Loading

codecov bot commented Dec 8, 2025 •

edited

Loading

PR-URL: #60991
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Daniel Lemire <daniel@lemire.me>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

PR-URL: #60991
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Daniel Lemire <daniel@lemire.me>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>