Repository navigation
Improve util.deprecate #1883
Description
As discussed in IRC recently with @Fishrock123 it would be nice if the deprecation messages contain a bit more info. Particulary I'd like to see
filename:linenumberwhere it happens in userland code. I think this could be parsed off the stack throughnew Error().stackor similar, suggestions welcome.- A prefix like
(node)to identify where the messages come from. - An flag like
-print-all-deprecationsto allow more than one print per deprecation to track down all places where it's happening.
Activity
thefourtheye commentedon Jun 3, 2015
Why do we need the prefix? Can you include an example where it would be useful?
silverwind commentedon Jun 3, 2015
Look at line 6 of https://gist.github.com/silverwind/a94589df8e3aae1907e1, do you think an outsider would know what to do based on that message, especially when they happen during something like
npm install.thefourtheye commentedon Jun 3, 2015
Oh okay. You mean
(request) ...? That would be good. About the line number and file name, what if the function is called more than once? Whenever user fixes one place he will get deprecation message for other places. Will that be okay?silverwind commentedon Jun 3, 2015
No, I mean a prefix that clearly identifies that the line was logged by node. For multiple prints: I want to keep the functionality to only prince once per function call, of course. Multiple messages should be opt-in through the flag.
Fishrock123 commentedon Jun 3, 2015
👍
thefourtheye commentedon Jun 3, 2015
@silverwind What if other libraries start using
util.deprecate? We cannot log(node)in that case, right?cjihrig commentedon Jun 3, 2015
Can't the existing
--trace-deprecationflag be used to accomplish your first item? The second item seems a bit unnecessary. +1 to the third item.silverwind commentedon Jun 3, 2015
@thefourtheye I think the intention is to move to an internal module eventually, @vkurchatkin had it moved in a recent PR if I recall correctly.
silverwind commentedon Jun 3, 2015
@cjihrig +1 on incorporating multiple prints into
--trace-deprecationvkurchatkin commentedon Jun 3, 2015
@silverwind no, it is just a helper function.
util.deprecateis untouched.This is already available with
--trace-deprecationThis could be just a part of the message
+0
silverwind commentedon Jun 3, 2015
Right, I didn't realize we were exposing this knowingly in the public API, so it's probably best to include the
(node)prefix in the messages itself.dougwilson commentedon Jun 6, 2015
People who want a public API should probably just use the
depdmodule or similar.Fishrock123 commentedon Jun 6, 2015
Right, but that doesn't really defer much from the fact that this already is public API.
The goal of what I was originally talking about was to better highlight spots where user code uses deprecated core APIs. However, I had forgotten about
--trace-deprecation, which I think does that.silverwind commentedon Jun 6, 2015
@dougwilson we gotta live with it being public now, or go through a tedious deprecation process of questionable benefit.
@Fishrock123 I didn't know about it either, but I think the default message should at least include the prefix and line number. The case of deprecation messages during
npm installis a good example. These would be just confusing without context.thefourtheye commentedon Jun 6, 2015
@silverwind The PR has the prefix change already. As we anyway extract the file name and line number from the stack trace, shall I just print them in normal deprecation warning as well?
thefourtheye commentedon Jun 7, 2015
I included a commit in the PR which will be actually printing the location where the deprecated item is used.
util: prepend '(node) ' to deprecation messages
silverwind commentedon Jul 3, 2015
Fixed by 9cd44bb with what was reasonable to change.
util: prepend '(node) ' to deprecation messages