Skip to content
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

domain: add --pending-deprecation warning #12519

Closed
wants to merge 1 commit into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Apr 19, 2017

Moves the domain module implementation to internal/domain.js so that core can continue to use it without triggering any dep warnings.

Add a pending deprecation warning that only emits when the --pending-deprecation flag is used.

/cc @nodejs/ctc

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

domain

Moves the domain module implementation to internal/domain.js so
that core can continue to use it without triggering any dep
warnings.

Add a pending deprecation warning that only emits when the
--pending-deprecation flag is used.
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. domain Issues and PRs related to the domain subsystem. events Issues and PRs related to the events subsystem / EventEmitter. repl Issues and PRs related to the REPL subsystem. labels Apr 19, 2017
@misterdjules
Copy link

misterdjules commented Apr 19, 2017

My understanding was that domains were deprecated, but mostly to document that they had problems. I also thought that we didn't have any other similar subsystem that is ready and that covers the same use cases.

If that is correct I would think we shouldn't emit any warning at runtime (regardless of whether it is a pending-deprecation warning or another type of warning), since we don't know how long they'll be around, and users who rely on the features provided by domains can't move to anything else.

In general I don't see a problem with just deprecating domains only in the documentation. It doesn't break users' code, and when something else that covers the same use cases is available, users can move to it whenever they're ready.

@jasnell
Copy link
Member Author

jasnell commented Apr 19, 2017

I'm not going to argue strongly in favor of this, but with efforts such as async-hooks beginning to take shape, the general idea is that a replacement for domain can be developed in user land on top of that, making it easier for us to begin taking more active steps. The use case for --pending-deprecation is specifically to use within CI environments where catching potentially troublesome uses of these kinds of APIs is most important. End users will generally never see these warnings.

@ChALkeR
Copy link
Member

ChALkeR commented Apr 19, 2017

@jasnell, is there a replacement that actually works atm? If not, I'm -1 on this.

The questions regarding why are domains doc-deprecated and what's the proposed replacement have arised multiple times already, and this will probably make those concerns stronger.

If there is a (for example user-land) replacement for domains which is already usable for all known used usecases, then +1.

@jasnell
Copy link
Member Author

jasnell commented Apr 19, 2017

@ChALkeR ... at the current time, not yet. And I definitely agree that having a viable working replacement is a good metric. If this is something that we generally do not yet want to see happen, then that's absolutely fine, I just want it to be a deliberate choice that we're waiting and not just left open and undecided.

@jasnell
Copy link
Member Author

jasnell commented Apr 20, 2017

Closing given the -1's

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. domain Issues and PRs related to the domain subsystem. events Issues and PRs related to the events subsystem / EventEmitter. repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants