Repository navigation
Should util.deprecate() be removed from the public API? #11642
Description
Reading from the docs, it is not clear whether util.deprecate() should be used by userland modules or not. There is the preamble that states:
The util module is primarily designed to support the needs of Node.js' own internal APIs. However, many of the utilities are useful for application and module developers as well
Reading this, I would guess that it is ok to use util.deprecate(). The output, however, looks like this:
(node:<strange number>) DeprecationWarning <message>
The (node) prefix is indicating that the deprecation warning is coming from node itself (introduced with 9cd44bb).
If util.deprecate() is meant for internal use only, I think it would be better to remove it from the official documentation because it can be misleading for both the library authors and library users.
Just as background: I was using this method for a breaking change in webpack's loader-utils. This module is used by almost every loader, so it is very likely that it appeared at least once in the console of every webpack user. I was using this method because I think it's good to have a unified way of handling deprecations, especially in combination with the command line flags provided by node. I just wanted to stick to the community default (at least as it appeared to me).
Judging from the feedback, however, many users didn't know where the message was coming from and what to do. And I think this is also due to the unfriendly appearance of the message. It is lacking the necessary information whether if it's a problem the developer must fix or the library author. You could use --trace-deprecation, but if you never heard of util.deprecate(), you don't know that this flag exists.
Having said this, this is what I would like to change:
- Either remove
util.deprecate()from the documentation or remove the(node)prefix - In any case: The output of the warning can be improved. For example, we could add the last one or two lines of the stack trace by default to reveal which library is causing the deprecation warning. We could also add an URL to the documentation that lists all the available cli flags. I think, elm is doing an incredible good job of explaining errors/problems.
Activity
sam-github commentedon Mar 2, 2017
I think we should deprecate
util.deprecate()(I know, a bit ciruclar) and then move it to lib/internal (where it can only be called by node) as soon as our deprecation policy allows.This is an example of exposing an API "because we made it for ourselves and it might be useful to someone else", and that exposure making it hard to change later to work better for Node.
dougwilson commentedon Mar 3, 2017
@jhnns FWIW I made https://github.com/dougwilson/nodejs-depd for use in the Express system and it seems to handle the issues you're bringing up, notably it's public API for module authors to use and it provides a location of the issue by default (and even honors the Node.js command line flags, to boot).
jhnns commentedon Mar 3, 2017
You can do that, but I think the output should still be improved. I'm seeing deprecation warnings every now and then in my console, and before I knew about the command line flags I was always like: "Oh, what's that? Is that an error? Where is it coming from?". I'm not the kind of developer who is comfortable with ignoring mysterious deprecation warnings 😁
As a developer, I'm concerned about the origin: Who is responsible for fixing this? And unless you've used
util.deprecate()before or you know all cli flags by heart ❤️, you have no clue how to locate that warning.Yeah, I think I've seen your module before, but at the time of writing I didn't remember it anymore 😁.
Personally, I would rather like to see this functionality in core because it's something every library has to deal with – and node's own deprecation warnings would be better. Additionally, the node user would benefit from a unified way of tracing deprecations/disabling deprecation warnings.
If you decide to deprecate this and remove it from the public API, I think it would be beneficial to link to
depdas userland alternative and to promote the CLI andprocessflags. Every deprecation library should honor these.seishun commentedon Mar 3, 2017
I agree with deprecating
util.deprecate(). I think it should be up to the module authors to decide how exactly the deprecation warning should look like, whether it should honor--no-deprecation, and so on.Also,
util.deprecate()being public means that any changes to its behavior would have to besemver-major. For example, let's say we decide to add information about flags to the deprecation message. That might break some module's tests.jasnell commentedon Mar 3, 2017
To explain the syntax just a bit...
The
<strange number>is thePIDof the node process. If you have multiple node processes running this number is helpful in differentiating which emitted the warning.The
[<deprecation code>]is a new addition. All Node.js generated deprecations will have a static identifier code in the formDEP00NNthat will identify the deprecation in question. These can be looked up the in the documentation for more context.At this point in time,
util.deprecate()is a thin wrapper around theprocess.emitWarning()API. It is possible for a user to emit their own deprecation warnings usingprocess.emitWarning('mesage', 'DeprecationWarning'). Theutil.deprecate()adds some additional logic around it to automatically handle function and class deprecations so that the message is only emitted once. As such, it definitely has utility and I have seen userland modules take advantage of it.I would be -1 at deprecating it at this point simply because it is being used effectively and there's really no cost to keeping it.
sam-github commentedon Mar 3, 2017
@jhnns Sorry, you mentioned too many unrelated things in this single issue. Besides what I said earlier, I think your suggestion of including a line or two of context is a decent one. I suggest you bring that suggestion up in a seperate issue (or PR it).
@jasnell I think the OP is contesting whether this API can be used effectively outside of node, given its output format.
jasnell commentedon Mar 3, 2017
@sam-github ... yes, I got that, and I have seen it used effectively. The addition of the
codeactually makes it significantly easier. For instance, one could doutil.deprecate(fn, 'message', 'MYDEPCODE')and the output would be:(node:<pid>) [MYDEPCODE] DeprecationWarning: message. The `MYDEPCODE then becomes a differentiator for userland deprecations.sam-github commentedon Mar 3, 2017
Without a registry of userland deprecation numbers, how will they allocate deprecation codes? Since the deprecation is identified as a node deprecation, I predict github issues opened on node for user-land deprecations coming soon. IMO, building this into node stifles innovation in the community.
dougwilson commentedon Mar 3, 2017
That was discussed and rejected previously in #4782 (comment)
jhnns commentedon Mar 5, 2017
What benefits come with this flexibility? That some modules will just ignore
--no-deprecation? I don't see how that is beneficial to the user. That's why I thought that this should be provided by core.Since there is no explanation in the message, I would have to read node's source code to find out that this is the process id. Furthermore, it feels strange to me that the process id is more important than parts of the stack trace.
Now I understand the reasoning behind it, but it's sad that the default is optimized to be parsed by machines instead to be read by humans. Because I'd assume that many warnings will be read by humans. And unless you've heard of
--trace-warningsor the warning API before, there is no clue how to deal with that message. I think it would already help to append something like Use the --trace-warnings command line flag to get a full stack trace of this warning.You're right. I've created a dedicated issue for that: #11703
This discussion should be about removing
util.deprecate()from the public API.[-]Clarify usage of util.deprecate()[/-][+]Should util.deprecate() be removed from the public API?[/+]Fishrock123 commentedon Mar 13, 2017
I think we should have a publicly supported way for user-land runtime deprecations.
addaleax commentedon Apr 29, 2017
Given that #12631 was closed, I’m going to close this too as “util.deprecate() is here to stay”. If you think we need to have more discussion on this, please feel free to re-open.