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 07220230d9 and the problem exacerbated in
8ba66c5e7b. Our team noticed this because our tests started failing
when attempting to update to Node.js 20.19.5.
Some recent commits, such as 1093f38c43 or 69453378fc, 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: https://github.com/nodejs/node/issues/59731
Fixes: https://github.com/nodejs/node/issues/58903
Refs: https://github.com/nodejs/node/pull/58709
Refs: https://github.com/nodejs/node/pull/58775
Refs: https://github.com/nodejs/node/pull/57909
Refs: https://github.com/nodejs/node/pull/58891
PR-URL: https://github.com/nodejs/node/pull/59774
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Dario Piotrowicz <dario.piotrowicz@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/58521
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
https://github.com/nodejs/node/pull/57909 introduced the disabling
of REPL tab completion on object containing proxies and getters
(since such completion triggers code evaluation which can be
unexpected/disruptive for the user)
the solution in 57909 did not address all possible such cases,
the changes here improve on such solution by using acorn and
AST analysis to cover most if not all possible cases
PR-URL: https://github.com/nodejs/node/pull/58891
Reviewed-By: James M Snell <jasnell@gmail.com>
prevent incorrect throws of `ERR_USE_AFTER_CLOSE` errors when the eval
function of a repl server returns an error after the repl server
has been closed
PR-URL: https://github.com/nodejs/node/pull/58791
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
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: https://github.com/nodejs/node/pull/58775
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
this commit reintroduces the REPL custom eval tests that have
been introduced in https://github.com/nodejs/node/pull/57691
but reverted in https://github.com/nodejs/node/pull/57793
the tests turned out problematic before because `getReplOutput`,
the function used to return the repl output wasn't taking into
account that input processing and output emitting are asynchronous
operation can resolve with a small delay
the new implementation here replaces `getReplOutput` with
`getReplRunOutput` that resolves repl inputs by running them
and using the repl prompt as an indicator to when the input
processing has completed
PR-URL: https://github.com/nodejs/node/pull/57850
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
the REPL completion logic evaluates code, this is generally ok
but it can be problematic when there are objects with nested
properties since the code evaluation would trigger their potential
getters (e.g. the `obj.foo.b` line would trigger the getter of
`foo`), the changes here disable the completion logic when proxies
and getters are involved thus making sure that code evaluation does
not take place when it can potentially (and surprisingly to the user)
trigger side effectful behaviors
PR-URL: https://github.com/nodejs/node/pull/57909
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This reverts commit 1f7cfb7a36, which was
merged into the main branch despite relevant test failures.
PR-URL: https://github.com/nodejs/node/pull/57793
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/57508
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Zijian Liu <lxxyxzj@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/54869
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: LiviaMedeiros <livia@cirno.name>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Specifically, `delete Array.prototype.lastIndexOf` immediately crashes
the REPL, as does deletion of a few other Array prototype methods.
PR-URL: https://github.com/nodejs/node/pull/31457
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
This change ensures that 'npm' within JavaScript code is not mistakenly
interpreted as an npm command when the error is recoverable.
This allows 'npm' to be treated as expected in such scenarios.
Fixes: https://github.com/nodejs/node/issues/54830
PR-URL: https://github.com/nodejs/node/pull/54848
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/52625
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
This commit adds a proper error message using ERR_MISSING_ARGS('file')
when a .save or .load REPL command is runned. This commit also adds
test for both of this cases.
Fixes: https://github.com/nodejs/node/issues/52218
Signed-off-by: Thomas Mauran <thomas.mauran@etu.umontpellier.fr>
PR-URL: https://github.com/nodejs/node/pull/52225
Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
Split the `internal/process/esm_loader` file which contains the
singleton cascaded loader:
- The the singleton cascaded loader now directly resides in
`internal/modules/esm/loader`, where the constructor also lives.
This file is the root of most circular dependency of ESM code,
(because components of the loader need the singleton itself),
so this makes the dependency more obvious. Added comments about
loading it lazily to avoid circular dependency.
- The getter to the cascaded loader is also turned into a method
to make the side effect explicit.
- The sequence of `loadESM()` and `handleMainPromise` is now merged
together into `runEntryPointWithESMLoader()` in
`internal/modules/run_main` because this is intended to run entry
points with the ESM loader and not just any module.
- Documents how top-level await is handled.
PR-URL: https://github.com/nodejs/node/pull/51999
Fixes: https://github.com/nodejs/node/issues/42868
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Expose the default prepareStackTrace implementation as
`Error.prepareStackTrace` so that userland can chain up formatting of
stack traces with built-in source maps support.
PR-URL: https://github.com/nodejs/node/pull/50827
Fixes: https://github.com/nodejs/node/issues/50733
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
The second parameter of `Error.prepareStackTrace` is an array of
reversed call site frames.
PR-URL: https://github.com/nodejs/node/pull/50827
Fixes: https://github.com/nodejs/node/issues/50733
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
PR-URL: https://github.com/nodejs/node/pull/50592
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: James M Snell <jasnell@gmail.com>
Instead of using the public versions of the vm APIs internally,
use the internal versions so that we can skip unnecessary
argument validation.
The public versions would need special care to the generation
of host-defined options to hit the isolate compilation cache
when imporModuleDynamically isn't used, while internally it's
almost always used, so this allows us to handle the host-defined
options separately.
PR-URL: https://github.com/nodejs/node/pull/50137
Refs: https://github.com/nodejs/node/issues/35375
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
The old import assertions proposal has been
renamed to "import attributes" with the follwing major changes:
1. The keyword is now `with` instead of `assert`.
2. Unknown assertions cause an error rather than being ignored,
This commit updates the documentation to encourage folks to use the new
syntax, and add aliases for module customization hooks.
PR-URL: https://github.com/nodejs/node/pull/50140
Fixes: https://github.com/nodejs/node/issues/50134
Refs: 159c82c5e6
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
When using .load the REPL would accumulate indentation with each line
including the indentation from all previous lines. Now it keeps the
indentation at the correct level.
Fixes: https://github.com/nodejs/node/issues/47673
PR-URL: https://github.com/nodejs/node/pull/49461
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
Bootstrap per-realm callbacks like `prepare_stack_trace_callback` in
the ShadowRealm. This enables stack trace decoration in the ShadowRealm.
PR-URL: https://github.com/nodejs/node/pull/47107
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
fixup: add support for `Object.create(null)`
fixup: extend to any 1-argument Object.create call
fixup: add tests
PR-URL: https://github.com/nodejs/node/pull/46083
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
