-
Notifications
You must be signed in to change notification settings - Fork 30.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
doc,lib: remove deprecated domain module #45839
base: main
Are you sure you want to change the base?
Conversation
Review requested:
|
description: Documentation-only deprecation. | ||
--> | ||
|
||
Type: Documentation-only |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to first runtime deprecate it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note my "draft PR to check citgm" remark (which is inconclusive so far - citgm seems to be in bad shape)
That said... the domain module has basically always been deprecated0. Standard procedures are nice but sometimes it makes no sense to follow them.
0 it was added in v0.10.0 and deprecated in v1.0.0. We'll not talk about the brown paper bag releases that were v0.12.x.
@@ -717,24 +717,6 @@ Type: Documentation-only | |||
The [`ecdh.setPublicKey()`][] method is now deprecated as its inclusion in the | |||
API is not useful. | |||
|
|||
### DEP0032: `node:domain` module |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to keep this entry, it should say Type: End-of-Life
, e.g. see
Lines 44 to 64 in 4712d60
### DEP0001: `http.OutgoingMessage.prototype.flush` | |
<!-- YAML | |
changes: | |
- version: v14.0.0 | |
pr-url: https://github.com/nodejs/node/pull/31164 | |
description: End-of-Life. | |
- version: | |
- v6.12.0 | |
- v4.8.6 | |
pr-url: https://github.com/nodejs/node/pull/10116 | |
description: A deprecation code has been assigned. | |
- version: v1.6.0 | |
pr-url: https://github.com/nodejs/node/pull/1156 | |
description: Runtime deprecation. | |
--> | |
Type: End-of-Life | |
`OutgoingMessage.prototype.flush()` has been removed. Use | |
`OutgoingMessage.prototype.flushHeaders()` instead. |
<a id="ERR_DOMAIN_CALLBACK_NOT_AVAILABLE"></a> | ||
|
||
### `ERR_DOMAIN_CALLBACK_NOT_AVAILABLE` | ||
|
||
The `node:domain` module was not usable since it could not establish the | ||
required error handling hooks, because | ||
[`process.setUncaughtExceptionCaptureCallback()`][] had been called at an | ||
earlier point in time. | ||
|
||
<a id="ERR_DOMAIN_CANNOT_SET_UNCAUGHT_EXCEPTION_CAPTURE"></a> | ||
|
||
### `ERR_DOMAIN_CANNOT_SET_UNCAUGHT_EXCEPTION_CAPTURE` | ||
|
||
[`process.setUncaughtExceptionCaptureCallback()`][] could not be called | ||
because the `node:domain` module has been loaded at an earlier point in time. | ||
|
||
The stack trace is extended to include the point in time at which the | ||
`node:domain` module had been loaded. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be moved to https://nodejs.org/api/errors.html#legacy-nodejs-error-codes instead of being removed.
I might be telling you something you already know, but just in case: You can open a PR like this against your own fork and then put EDIT: Added some additional labels to hopefully further dissuade people from thinking this is ready for review. Feel free to remove them at any time. |
(I've removed the |
@bnoordhuis see #55204, that should handle the REPL. Once that lands, are you still interested in persueing this PR? |
Fixes: #45824
Draft PR to check citgm. The REPL was leaning quite heavily on domains so REPL tests are expected to fail. I didn't want to spend time on that for a trial balloon.
I deleted
lib/domain.js
for now but in the final version it should probably throw an exception (not emit a warning.)https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/3060/