Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
repl: fix getters triggering side effects during completion
  • Loading branch information
dario-piotrowicz committed Dec 13, 2025
commit 710817907cd91f23fe63c25fe8f01b7165097dbb
55 changes: 29 additions & 26 deletions lib/internal/repl/completion.js
Original file line number Diff line number Diff line change
Expand Up @@ -732,16 +732,20 @@ function includesProxiesOrGetters(expr, exprStr, evalFn, ctx, callback) {
if (astProp.type === 'Literal') {
// We have something like `obj['foo'].x` where `x` is the literal

if (safeIsProxyAccess(obj, astProp.value)) {
return cb(true);
}

const propDescriptor = ObjectGetOwnPropertyDescriptor(
obj,
`${astProp.value}`,
);
const propHasGetter = typeof propDescriptor?.get === 'function';
return cb(propHasGetter);
if (propHasGetter) {
return cb(true);
}

if (isProxy(obj[astProp.value])) {
return cb(true);
}

return cb(false);
}

if (
Expand All @@ -750,16 +754,20 @@ function includesProxiesOrGetters(expr, exprStr, evalFn, ctx, callback) {
) {
// We have something like `obj.foo.x` where `foo` is the identifier

if (safeIsProxyAccess(obj, astProp.name)) {
return cb(true);
}

const propDescriptor = ObjectGetOwnPropertyDescriptor(
obj,
`${astProp.name}`,
);
const propHasGetter = typeof propDescriptor?.get === 'function';
return cb(propHasGetter);
if (propHasGetter) {
return cb(true);
}

if (isProxy(obj[astProp.name])) {
return cb(true);
}

return cb(false);
}

return evalFn(
Expand All @@ -773,36 +781,31 @@ function includesProxiesOrGetters(expr, exprStr, evalFn, ctx, callback) {
getREPLResourceName(),
(err, evaledProp) => {
if (err) {
return callback(false);
return cb(false);
}

if (typeof evaledProp === 'string') {
if (safeIsProxyAccess(obj, evaledProp)) {
return cb(true);
}

const propDescriptor = ObjectGetOwnPropertyDescriptor(
obj,
evaledProp,
);
const propHasGetter = typeof propDescriptor?.get === 'function';
return cb(propHasGetter);
if (propHasGetter) {
return cb(true);
}

if (isProxy(obj[evaledProp])) {
return cb(true);
}

return cb(false);
}

return callback(false);
return cb(false);
},
);
}

function safeIsProxyAccess(obj, prop) {
// Accessing `prop` may trigger a getter that throws, so we use try-catch to guard against it
try {
return isProxy(obj[prop]);
} catch {
return false;
}
}

return callback(false);
}

Expand Down
27 changes: 27 additions & 0 deletions test/parallel/test-repl-completion-on-getters-disabled.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,33 @@ describe('REPL completion in relation of getters', () => {
['objWithGetters[getGFooKey()].b', []],
]);
});

test('no side effects are triggered for getters during completion', async () => {
const { replServer } = startNewREPLServer();

await new Promise((resolve, reject) => {
replServer.eval('const foo = { get name() { globalThis.nameGetterRun = true; throw new Error(); } };',
replServer.context, '', (err) => {
if (err) {
reject(err);
} else {
resolve();
}
});
});

['foo.name.', 'foo["name"].'].forEach((test) => {
replServer.complete(
test,
common.mustCall((error, data) => {
// The context's nameGetterRun variable hasn't been set
assert.strictEqual(replServer.context.nameGetterRun, undefined);
// No errors has been thrown
assert.strictEqual(error, null);
})
);
});
});
});

describe('completions on proxies', () => {
Expand Down
36 changes: 0 additions & 36 deletions test/parallel/test-repl-tab-complete-getter-error.js

This file was deleted.

Loading