doc: note util.isError() @@toStringTag limitations #5414
Conversation
mscdex
added
util
| `@@toStringTag`. | ||
|
|
||
| ```js | ||
| // This example requires the `--harmony-tostring` flag |
There was a problem hiding this comment.
Should we even document something based on a feature which is behind a flag?
There was a problem hiding this comment.
Normally, I'd say no. But this is already possible with things like babel-node, and is only a matter of time until it's possible without a flag (http://v8project.blogspot.com/2016/01/v8-release-49.html).
There was a problem hiding this comment.
Aren't we deprecating all these functions with #1301 (hopefully by v6.0)?
There was a problem hiding this comment.
@thefourtheye asks:
Aren't we deprecating all these functions with #1301 (hopefully by v6.0)?
Deprecated functions still need to be accurately documented. In this particular case, if the additional material provided here makes it clear that the function is not bullet-proof, then we're doing people a favor. (I'm not actually sure how likely it is that someone will run across the problem described here, but it's an awfully big ecosystem out there...)
There was a problem hiding this comment.
Yes. They are already deprecated in the docs. I think the warning is still worthwhile until they are no longer exposed from core.
There was a problem hiding this comment.
Okay, in that case, this particular thing is applicable to few other functions in util. Can we put this as a separate section and link it in all the relevant function descriptions?
There was a problem hiding this comment.
It should only still be applicable to isError(). The others were moved to checks at the C++ level.
There was a problem hiding this comment.
Oops, sorry. I should have checked that first.
|
LGTM |
util.isError() is the only remaining util.is*() method that depends on Object.prototype.toString() behavior. This commit notes the limitations of isError() related to @@toStringTag. Refs: nodejs#2201 PR-URL: nodejs#5414 Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
|
Landed in 54cbf28 |
|
This is applicable to LTS correct? |
|
Don't believe this one is relevant for LTS |
|
Not applicable to LTS |
One of the util.isError() examples states that a harmony flag is required. As of v6.0.0, this is no longer true. This commit removes the out of date reference. Refs: nodejs#5414
One of the util.isError() examples states that a harmony flag is required. As of v6.0.0, this is no longer true. This commit removes the out of date reference. Refs: nodejs#5414 PR-URL: nodejs#6486 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Pull Request check-list
make -j8 test(UNIX) orvcbuild test nosign(Windows) pass withthis change (including linting)?
test (or a benchmark) included?
existing APIs, or introduces new ones)?
Affected core subsystem(s)
doc
Description of change
util.isError()is the only remainingutil.is*()method that depends onObject.prototype.toString()behavior. This commit notes the limitations ofisError()related to@@toStringTag.Refs: #2201