lib,src: isInsideNodeModules should test on the first non-internal frame · nodejs/node@d438062 · GitHub
Skip to content

Commit d438062

Browse files
legendecasaduh95
legendecas
authored and
aduh95
committed
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>
1 parent 170fda5 commit d438062

8 files changed

Lines changed: 58 additions & 29 deletions

File tree

‎benchmark/internal/util_isinsidenodemodules.js‎

Lines changed: 2 additions & 3 deletions

‎lib/buffer.js‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -180,15 +180,14 @@ function showFlaggedDeprecation() {
180180
if (bufferWarningAlreadyEmitted ||
181181
++nodeModulesCheckCounter > 10000 ||
182182
(!require('internal/options').getOptionValue('--pending-deprecation') &&
183-
isInsideNodeModules(100, true))) {
183+
isInsideNodeModules(3))) {
184184
// We don't emit a warning, because we either:
185185
// - Already did so, or
186186
// - Already checked too many times whether a call is coming
187187
// from node_modules and want to stop slowing down things, or
188188
// - We aren't running with `--pending-deprecation` enabled,
189189
// and the code is inside `node_modules`.
190-
// - We found node_modules in up to the topmost 100 frames, or
191-
// there are more than 100 frames and we don't want to search anymore.
190+
// - If the topmost non-internal frame is not inside `node_modules`.
192191
return;
193192
}
194193

‎lib/internal/modules/cjs/loader.js‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1538,9 +1538,8 @@ function loadESMFromCJS(mod, filename, format, source) {
15381538
} else if (mod[kIsCachedByESMLoader]) {
15391539
// It comes from the require() built for `import cjs` and doesn't have a parent recorded
15401540
// in the CJS module instance. Inspect the stack trace to see if the require()
1541-
// comes from node_modules and reduce the noise. If there are more than 100 frames,
1542-
// just give up and assume it is under node_modules.
1543-
shouldEmitWarning = !isInsideNodeModules(100, true);
1541+
// comes from node_modules as a direct call and reduce the noise.
1542+
shouldEmitWarning = !isInsideNodeModules();
15441543
}
15451544
} else {
15461545
shouldEmitWarning = true;

‎lib/punycode.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ const {
33
isInsideNodeModules,
44
} = internalBinding('util');
55

6-
if (!isInsideNodeModules(100, true)) {
6+
if (!isInsideNodeModules()) {
77
process.emitWarning(
88
'The `punycode` module is deprecated. Please use a userland ' +
99
'alternative instead.',

‎lib/url.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ const {
131131
let urlParseWarned = false;
132132

133133
function urlParse(url, parseQueryString, slashesDenoteHost) {
134-
if (!urlParseWarned && !isInsideNodeModules(100, true)) {
134+
if (!urlParseWarned && !isInsideNodeModules(2)) {
135135
urlParseWarned = true;
136136
process.emitWarning(
137137
'`url.parse()` behavior is not standardized and prone to ' +

‎src/node_util.cc‎

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -320,23 +320,21 @@ static void GetCallSites(const FunctionCallbackInfo<Value>& args) {
320320
args.GetReturnValue().Set(callsites);
321321
}
322322

323+
/**
324+
* Checks whether the current call directly initiated from a file inside
325+
* node_modules. This checks up to `frame_limit` stack frames, until it finds
326+
* a frame that is not part of node internal modules.
327+
*/
323328
static void IsInsideNodeModules(const FunctionCallbackInfo<Value>& args) {
324329
Isolate* isolate = args.GetIsolate();
325-
CHECK_EQ(args.Length(), 2);
326-
CHECK(args[0]->IsInt32()); // frame_limit
327-
// The second argument is the default value.
328330

329-
int frames_limit = args[0].As<v8::Int32>()->Value();
331+
int frames_limit = (args.Length() > 0 && args[0]->IsInt32())
332+
? args[0].As<v8::Int32>()->Value()
333+
: 10;
330334
Local<StackTrace> stack =
331335
StackTrace::CurrentStackTrace(isolate, frames_limit);
332336
int frame_count = stack->GetFrameCount();
333337

334-
// If the search requires looking into more than |frames_limit| frames, give
335-
// up and return the specified default value.
336-
if (frame_count == frames_limit) {
337-
return args.GetReturnValue().Set(args[1]);
338-
}
339-
340338
bool result = false;
341339
for (int i = 0; i < frame_count; ++i) {
342340
Local<StackFrame> stack_frame = stack->GetFrame(isolate, i);
@@ -350,13 +348,11 @@ static void IsInsideNodeModules(const FunctionCallbackInfo<Value>& args) {
350348
if (script_name_str.starts_with("node:")) {
351349
continue;
352350
}
353-
if (script_name_str.find("/node_modules/") != std::string::npos ||
354-
script_name_str.find("\\node_modules\\") != std::string::npos ||
355-
script_name_str.find("/node_modules\\") != std::string::npos ||
356-
script_name_str.find("\\node_modules/") != std::string::npos) {
357-
result = true;
358-
break;
359-
}
351+
result = script_name_str.find("/node_modules/") != std::string::npos ||
352+
script_name_str.find("\\node_modules\\") != std::string::npos ||
353+
script_name_str.find("/node_modules\\") != std::string::npos ||
354+
script_name_str.find("\\node_modules/") != std::string::npos;
355+
break;
360356
}
361357

362358
args.GetReturnValue().Set(result);

‎test/parallel/test-internal-util-isinsidenodemodules.js‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
'use strict';
2+
3+
// Flags: --expose-internals
4+
5+
const common = require('../common');
6+
const assert = require('assert');
7+
const vm = require('vm');
8+
9+
const { internalBinding } = require('internal/test/binding');
10+
const { isInsideNodeModules } = internalBinding('util');
11+
12+
const script = new vm.Script(`
13+
const runInsideNodeModules = (cb) => {
14+
return cb();
15+
};
16+
17+
runInsideNodeModules;
18+
`, {
19+
filename: '/workspace/node_modules/test.js',
20+
});
21+
const runInsideNodeModules = script.runInThisContext();
22+
23+
// Test when called directly inside node_modules
24+
assert.strictEqual(runInsideNodeModules(isInsideNodeModules), true);
25+
26+
// Test when called inside a user callback from node_modules
27+
runInsideNodeModules(common.mustCall(() => {
28+
function nonNodeModulesFunction() {
29+
assert.strictEqual(isInsideNodeModules(), false);
30+
}
31+
32+
nonNodeModulesFunction();
33+
}));
34+
35+
// Test when called outside node_modules
36+
assert.strictEqual(isInsideNodeModules(), false);

‎typings/internalBinding/util.d.ts‎

Lines changed: 1 addition & 1 deletion

0 commit comments

Comments
 (0)