Skip to content

repl: improve tab completion on computed properties - #58775

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
dario-piotrowicz:dario/improve-tab-completion-on-computed-props
Jun 26, 2025
Merged

repl: improve tab completion on computed properties#58775
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
dario-piotrowicz:dario/improve-tab-completion-on-computed-props

Conversation

@dario-piotrowicz

@dario-piotrowiczdario-piotrowicz commented Jun 21, 2025

Copy link
Copy Markdown
Member

improve the tab completion capabilities around computed properties by replacing the use of brittle and error prone Regex checks with more robust AST based analysis


Followup from #58709

@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. repl Issues and PRs related to the REPL subsystem. labels Jun 21, 2025
Comment threadlib/repl.js Outdated
Comment on lines 1483 to 1492

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I'm removing this comment since I think that it is inaccurate (and as a consequence misleading), since various type of lines pass through here, like for example { a: true }

I think/hope that the code structure makes it clear enough what this else if block is for (I also have half a mind to do some refactoring to also make things clearer later on 🤔)

@dario-piotrowicz
dario-piotrowiczforce-pushed the dario/improve-tab-completion-on-computed-props branch from b502583 to fdfdac0CompareJune 21, 2025 12:20
improve the tab completion capabilities around computed properties
by replacing the use of brittle and error prone Regex checks with
more robust AST based analysis
@dario-piotrowicz
dario-piotrowiczforce-pushed the dario/improve-tab-completion-on-computed-props branch from fdfdac0 to 1398800CompareJune 21, 2025 12:23
@codecov

codecovBot commented Jun 21, 2025

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 89.10891% with 11 lines in your changes missing coverage. Please review.

Project coverage is 90.09%. Comparing base (c3b9868) to head (1398800).
Report is 168 commits behind head on main.

Files with missing linesPatch %Lines
lib/repl.js89.10%10 Missing and 1 partial ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #58775 +/- ##
==========================================
- Coverage 90.13% 90.09% -0.04% 
==========================================
Files 640 640 Lines 188294 188399 +105 Branches 36923 36945 +22 ==========================================
+ Hits 169712 169747 +35 - Misses 11304 11354 +50 - Partials 7278 7298 +20 
Files with missing linesCoverage Δ
lib/repl.js94.78% <89.10%> (-0.09%)⬇️

... and 24 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

Copy link
Copy Markdown
Collaborator

@dario-piotrowiczdario-piotrowicz added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 21, 2025
@anonriganonrig added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 26, 2025
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 26, 2025
@nodejs-github-bot
nodejs-github-bot merged commit 8ba66c5 into nodejs:mainJun 26, 2025
28 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 8ba66c5

@dario-piotrowicz
dario-piotrowicz deleted the dario/improve-tab-completion-on-computed-props branch June 26, 2025 10:38
targos pushed a commit that referenced this pull request Jul 3, 2025
improve the tab completion capabilities around computed properties
by replacing the use of brittle and error prone Regex checks with
more robust AST based analysis
PR-URL: #58775
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@aduh95aduh95 added the backport-requested-v22.x PRs awaiting manual backport to the v22.x-staging branch. label Jul 21, 2025
@aduh95

Copy link
Copy Markdown
Contributor

This doesn't land cleanly on v22.x-staging, we'd need a backport PR if we want it on v22.x

addaleax added a commit to addaleax/node that referenced this pull request Sep 7, 2025
A number of recent changes to the REPL tab completion logic have
introduced the ability for completion to cause side effects,
specifically, calling arbitrary functions or variable
assignments/updates.
This was first introduced in 0722023 and the problem exacerbated in
8ba66c5. Our team noticed this because our tests started failing
when attempting to update to Node.js 20.19.5.
Some recent commits, such as 1093f38 or 6945337, have
messages or PR descriptions that imply the intention to avoid side
effects, which I can can generally be agreed upon is in line with the
expectations that a user has of autocomplete functionality.
However, some of the tests introduced in those commts specifically
verify that side effects *can* happen under specific circunmstances.
I am assuming here that this is unintentional, and the corresponding
tests have been removed/replaced in this commit.
Fixes: nodejs#59731Fixes: nodejs#58903
Refs: nodejs#58709
Refs: nodejs#58775
Refs: nodejs#57909
Refs: nodejs#58891
nodejs-github-bot pushed a commit that referenced this pull request Sep 8, 2025
A number of recent changes to the REPL tab completion logic have
introduced the ability for completion to cause side effects,
specifically, calling arbitrary functions or variable
assignments/updates.
This was first introduced in 0722023 and the problem exacerbated in
8ba66c5. Our team noticed this because our tests started failing
when attempting to update to Node.js 20.19.5.
Some recent commits, such as 1093f38 or 6945337, have
messages or PR descriptions that imply the intention to avoid side
effects, which I can can generally be agreed upon is in line with the
expectations that a user has of autocomplete functionality.
However, some of the tests introduced in those commts specifically
verify that side effects *can* happen under specific circunmstances.
I am assuming here that this is unintentional, and the corresponding
tests have been removed/replaced in this commit.
Fixes: #59731Fixes: #58903
Refs: #58709
Refs: #58775
Refs: #57909
Refs: #58891
PR-URL: #59774
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Dario Piotrowicz <dario.piotrowicz@gmail.com>
targos pushed a commit that referenced this pull request Sep 9, 2025
A number of recent changes to the REPL tab completion logic have
introduced the ability for completion to cause side effects,
specifically, calling arbitrary functions or variable
assignments/updates.
This was first introduced in 0722023 and the problem exacerbated in
8ba66c5. Our team noticed this because our tests started failing
when attempting to update to Node.js 20.19.5.
Some recent commits, such as 1093f38 or 6945337, have
messages or PR descriptions that imply the intention to avoid side
effects, which I can can generally be agreed upon is in line with the
expectations that a user has of autocomplete functionality.
However, some of the tests introduced in those commts specifically
verify that side effects *can* happen under specific circunmstances.
I am assuming here that this is unintentional, and the corresponding
tests have been removed/replaced in this commit.
Fixes: #59731Fixes: #58903
Refs: #58709
Refs: #58775
Refs: #57909
Refs: #58891
PR-URL: #59774
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Dario Piotrowicz <dario.piotrowicz@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.backport-requested-v22.xPRs awaiting manual backport to the v22.x-staging branch.needs-ciPRs that need a full CI run.replIssues and PRs related to the REPL subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@dario-piotrowicz@nodejs-github-bot@aduh95@jasnell@anonrig