repl: deprecate repl.builtinModules - #57508
Merged
nodejs-github-bot merged 1 commit intoApr 5, 2025
Merged
Conversation
Copy link
Copy Markdown
Member
Sorry, something went wrong.
dario-piotrowicz
force-pushed
the
dario/57504/deprecate-repl-builtinmodules
branch
7 times, most recently
from
39271fa to
f38edf8
Compare
March 16, 2025 22:45
juanarbol
approved these changes
Mar 16, 2025
juanarbol
left a comment
Copy link
Copy Markdown
Member
There was a problem hiding this comment.
Amazing, thanks! If CI is happy, I'm happy as well.
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
|
Can you please open a PR that first doc-deprecates the API? Hum actually, I'm not sure whether --pending-deprecation changes would be semver-major, so that might be fine EDIT2: just checked, we have precedents for landing as semver-patch pending deprecations |
Sorry, something went wrong.
aduh95
reviewed
Mar 16, 2025
Comment thread
doc/api/repl.md
Outdated
aduh95
reviewed
Mar 16, 2025
Comment thread
doc/api/repl.md
Outdated
This comment was marked as outdated.
aduh95
reviewed
Mar 17, 2025
Comment thread
doc/api/deprecations.md
Outdated
aduh95
approved these changes
Mar 17, 2025
aduh95
approved these changes
Mar 17, 2025
jasnell
approved these changes
Mar 17, 2025
Copy link
Copy Markdown
Collaborator
Sorry, something went wrong.
Copy link
Copy Markdown
Codecov ReportAll modified and coverable lines are covered by tests ✅ Additional details and impacted files @@ Coverage Diff @@
## main #57508 +/- ##
=======================================
Coverage 90.22% 90.23%
=======================================
Files 630 630
Lines 185055 185063 +8
Branches 36216 36221 +5
=======================================
+ Hits 166975 166989 +14
- Misses 11042 11043 +1
+ Partials 7038 7031 -7
... and 29 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Sorry, something went wrong.
meixg
approved these changes
Mar 17, 2025
Lxxyx
reviewed
Mar 17, 2025
Comment thread
lib/repl.js
Outdated
BridgeAR
requested changes
Mar 17, 2025
BridgeAR
left a comment
Copy link
Copy Markdown
Member
There was a problem hiding this comment.
I would actually like to keep this around and just fix the issue. The reason is that we expose underscored modules in Module one and those should never be used.
The current issue with the repl exposed one is that it filters node: prefixed ones out. That should not be the case.
We can also update the documentation to say "all besides underscored'.
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
Any particular reason for that? Do you have a use-case in mind? |
Sorry, something went wrong.
dario-piotrowicz
force-pushed
the
dario/57504/deprecate-repl-builtinmodules
branch
from
37cfc74 to
e9fd52f
Compare
March 25, 2025 00:08
BridgeAR
approved these changes
Mar 25, 2025
BridgeAR
left a comment
Copy link
Copy Markdown
Member
There was a problem hiding this comment.
LGTM
I do overall agree that this should not exist on the repl, since we already have it exposed elsewhere.
I suggest to deprecate the runtime usage of the underscored modules as well in different PRs.
Sorry, something went wrong.
Copy link
Copy Markdown
Collaborator
Commit Queue failed- Loading data for nodejs/node/pull/57508 ✔ Done loading data for nodejs/node/pull/57508 ----------------------------------- PR info ------------------------------------ Title repl: deprecate `repl.builtinModules` (#57508) Author Dario Piotrowicz <dario.piotrowicz@gmail.com> (@dario-piotrowicz) Branch dario-piotrowicz:dario/57504/deprecate-repl-builtinmodules -> nodejs:main Labels repl, author ready, needs-ci Commits 1 - repl: deprecate `repl.builtinModules` Committers 1 - Dario Piotrowicz <dario@cloudflare.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> ------------------------------ Generated metadata ------------------------------ 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> -------------------------------------------------------------------------------- ℹ This PR was created on Sun, 16 Mar 2025 22:36:51 GMT ✔ Approvals: 6 ✔ - Juan José Arboleda (@juanarbol): https://github.com/nodejs/node/pull/57508#pullrequestreview-2688852607 ✔ - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/57508#pullrequestreview-2688878116 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/57508#pullrequestreview-2688878596 ✔ - Xuguang Mei (@meixg): https://github.com/nodejs/node/pull/57508#pullrequestreview-2689145592 ✔ - Ruben Bridgewater (@BridgeAR) (TSC): https://github.com/nodejs/node/pull/57508#pullrequestreview-2712875322 ✔ - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/57508#pullrequestreview-2696025170 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2025-03-17T00:31:48Z: https://ci.nodejs.org/job/node-test-pull-request/65794/ ⚠ Commits were pushed after the last Full PR CI run: ⚠ - repl: deprecate `repl.builtinModules` - Querying data for job/node-test-pull-request/65794/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncuhttps://github.com/nodejs/node/actions/runs/14261203802 |
Sorry, something went wrong.
Lxxyx
approved these changes
Apr 4, 2025
Copy link
Copy Markdown
Collaborator
Sorry, something went wrong.
Copy link
Copy Markdown
Collaborator
Sorry, something went wrong.
RafaelGSS
pushed a commit
that referenced
this pull request
May 1, 2025
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com> PR-URL: #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>
RafaelGSS
pushed a commit
that referenced
this pull request
May 2, 2025
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com> PR-URL: #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>
aduh95
added a commit
that referenced
this pull request
May 6, 2025
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com> PR-URL: #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>
RafaelGSS
pushed a commit
that referenced
this pull request
May 14, 2025
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com> PR-URL: #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>
aduh95
added a commit
that referenced
this pull request
May 16, 2025
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com> PR-URL: #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>
aduh95
added a commit
that referenced
this pull request
May 17, 2025
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com> PR-URL: #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>
aduh95
added a commit
that referenced
this pull request
May 19, 2025
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com> PR-URL: #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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #57504