repl: deprecate duplicated repl features

[BridgeAR]

repl: deprecate repl.inputStream and repl.outputStream

The stream is exposed twice. As such it's best to rely upon the
.input and .output properties set by readline.

repl: deprecate repl._builtinLibs

This is a manually edited and outdated list of builtin modules.
Instead, it is better to rely upon the officially documented way
to get a list of builtin modules.

repl: remove obsolete completer variable

It is also assigned in readline. There is not need to assign the
variable twice.

repl: rewrite exports to module.exports

This makes sure all exports are in one place. Thus, it is easier to
see what parts are actually exported and which are not.

@nodejs/repl @nodejs/tsc PTAL

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/31227/
CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2374/

[addaleax]

Our deprecation policy is – luckily – quite explicit now about the fact that aliases should be preferred to runtime deprecations.

As for _builtinLibs, the mismatches with builtinModules are intentional (on my end – can’t speak for you or others, obviously), partly in order to sort out the halfway public underscored modules like _http_outgoing, _tls_common, etc.

[addaleax]

Please have a look at the individual commit messages for details.

By the way:

$ git config --global alias.desc '!f() { git log --pretty="format:##### %s%n%n%b" upstream/master...HEAD --reverse; }; f'
$ git desc

Gives you a “nice” markdown printout of the commits on the current branch. I find it quite helpful when putting together PR descriptions for easier review.

[BridgeAR]

@addaleax thanks for the nice shortcut 👍

I just updated the deprecation to a doc-only deprecation which is allowed for aliases.

The builtinLibs are only exposed in the REPL and they are not up to date. They lack a few modules that should be listed. I think we should also update builtinModules to only list "actual" modules and not the underscored ones.

[addaleax]

@BridgeAR The two module lists have very different purposes:

• repl._builtinLibs gives the list of modules that should be easily accessible in the REPL.
• module.builtinModules gives the list of modules for which require(moduleName) will load a Node.js core module instead of one from the disk.

I think they’re fundamentally irreconceilable.

[BridgeAR]

If we keep _builtinLibs we should document it and name it builtinLibs. And builtinLibs should always rely upon builtinModules, otherwise we have an outdated list as we currently have (multiple modules are missing). It is possible to filter the underscores etc. that's actually what I do in #33282.

[addaleax]

@BridgeAR That sounds like a good approach to me too 👍

[BridgeAR]

@addaleax I would still deprecate _builtinLibs though. I am going to open a separate PR to add builtinLibs.

[BridgeAR]

@addaleax I just opened the other PR to document the property properly. Since I changed the deprecation to a doc-deprecation, is there still something that you disagree with?

[BridgeAR]

Ping @addaleax

[addaleax]

@BridgeAR I think this still runtime-deprecates _buildinLibs? Once we have #33295 – which I’m a fan of, obviously – it could just be an alias.

[BridgeAR]

@addaleax I changed it to a documentation-only deprecation as well. PTAL.

[BridgeAR]

I just updated the semverness to semver-minor, nice our deprecation documentation explicitly just mentions runtime and end-of-life deprecations being semver-major.

[Documentation-only] deprecations [...] are not breaking changes (semver-major)

[targos]

Landed in 2b50cd7...ab7d520

[aduh95]

@targos This has landed on master without changing the DEPXXXX to the actual ones (should be DEP0141 and DEP0142 I think):

node/doc/api/deprecations.md

Line 2671 in 48b1595

<a id="DEP0XXX"></a>

node/doc/api/deprecations.md

Line 2685 in 48b1595

<a id="DEP0XX1"></a>