-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
Request to undeprecate util.is* functions #43715
Comments
Context for deprecation: #2447 |
Oh, I'm not the only one! There are people saying pretty much the same on the thread, even more eloquently: #2447 (comment) Basically, our message is:
|
I'm not sure why though since there are native ways to check for all of these? The problem is that something like Having our own similar but with possibly slightly different semantics methods for these things isn't really a positive in a time where we want to move to more standard JavaScript. |
When one sees When one sees plain In other words, typeof value == ' number' is tricky because it might not behave as you expect in all javascript environments. util.isNumber encapsulates that knowledge and implements the minimally sufficient check to detect the number for Node.js specifically (granted that we only use V8). You can argue that one should remember what checks work for Node.js vs what checks are necessary for all other environments out there, but that doesn't scale, it would incur too much extra cognitive load. |
I would argue that |
The |
Still, Imagine that you actually have to remember and type out the entire |
@WillAvudim I wrote the last comment after I checked the code and The thing is -it isn't entirely clear what the semantics are for these utilities. Let's take
I can come up with 10 more such questions for each helper basically :D |
You just listed even more reasons to encapsulate all these decisions in a single standard If you deprecate it, all of us would have to answer all these questions anew, and any Let's make everyone type out |
The deprecation of this is around for more than 10 major versions and I could not find a precedent where people were unhappy about it before in the last six years. As such, it is extremly unlikely to ever be changed again. |
I definitely see a precedent in the change thread, and I believe, the majority of Node.js users would fully support this undeprecation (#2447 (comment)). Do you have some sort of a voting mechanism to expose this decision publicly? I disagree that it should be a javascript feature, runtime is the right place for it since we know that we only use V8 and can leverage this knowledge to make some checks more efficient. |
In a gist: Generally every collaborator can block or land code changes, if the change is objected to discussion is had until consensus is reached, if consensus cannot be reached the technical steering committee gets involved and if they can't agree there is a majority vote. Community feedback is gathered through discussions, surveys etc. You can read more about the process in the repo docs. As for the technical bits:
I listed reasons why we cannot reasonably encapsulate these decisions in a single standard
i'm... not sure what you mean. TC39 specifies ECMAScript which defines semantics for how code should behave not how that behavior should be implemented. The fact we use V8 is irrelevant to that. |
The check All these util functions have been around for long, they are widely used, well understood, super useful, let's consider it all settled at this point. Semantic ambiguity is resolved by documenting how exactly the check is performed. Why do you want to delete them? Thank you for your answers! I'll look into those documents. |
Do we have data to back this up?
Almost certainly V8 devs would do a better job at optimizing such an util function at the engine level than we would in Node.js at the JavaScript level. And same would apply for other JS engines. |
241,660 references on github alone - https://github.com/search?p=99&q=util.isObject&type=Code I hope V8's TurboFan inlines it, so it just becomes a built-in check. May be they already do have some optimizations for typeof <> === "object", though that's beyond my expertise. |
There are a lot of false positives. That search matches anything that uses the string "util" and the string "isObject" but it need not be "util.isObject". A better (but still not correct) search is https://github.com/search?q=%22util.isObject%22&type=Code but that will match "util/isObject" etc. The GitHub search does not match punctuation. So, OK, that still matches 221,279 results. But that includes markdown and HTML and anything. And that also includes a lot of very old code. The search for "new Buffer()" has even more JavaScript results than "util.isObject" so I'm not sure that these GitHub search results are going to win the day here. |
I would recommend https://sourcegraph.com/search or https://cs.github.com, they allow way more accurate searching. |
I agree with this assessment from @BridgeAR. I would be strongly opposed to changing this. Use of these functions should be discouraged. There are idiomatic JavaScript ways to do these checks that allow you to make it clear what you're doing (e.g., are you excluding |
@BridgeAR assessment was based on the assumption that e.g.
Let's judge the change by the outcome it would produce, not by dogmatic beliefs of the past. Sure, mistakes have been made, the functions have been marked as deprecated, dissent ignored. That doesn't mean that we want to make everyone write their own version of said functions, or endlessly repeat the same recondite formulas like
Judging by the existing codebases, people tend to use Sure, you can delete these functions, break lots of code in the wild, force everyone incorporate a clone of these functions into their codebases, and all for what? What are the benefits you are after? In what real-world scenario would you not check for |
No one is suggesting that is ever going to happen. Deprecation means discouraged, not slated for removal. |
Oh, they shall never be deleted? I see. I'm sorry, I acted on the assumption that deprecation indicates imminent removal. This reminds me Google's modus operandi: Things are either deprecated or not yet implemented. Closing this bug then. Thank you for all your responses and have a wonderful day! |
To be fair, that's a common source of confusion and we (Node.js) could do more to clarify. |
We could use the |
I thought that the Legacy status specifically indicates that Node.js provides a new API, which isn't the case in this situation, but now that I look at the text, it just says "other APIs are available" which I guess doesn't mean necessarily APIs that Node.js provides. And even if it does, we can change the text. |
What is the problem this feature will solve?
I'd like to request to undeprecate all
util.is*
functions currently marked for deprecation, and keep them intact in all future versions of Node.js.What is the feature you are proposing to solve the problem?
Reasons:
util.is*
functions (util.isObject
,util.isNumber
,util.isRegExp
) are immensely useful and used throughout the codebase of pretty much every Node.js app.They are succinct and help to avoid unnecessary cognitive load, while the suggested alternatives are verbose and difficult to remember, and write correctly every single time.
Take for example
util.isObject
and compare the functionisObject
:VS what its documentation suggests I'd have to write instead:
Do you really think that the second fragment is more obvious to any software engineer out there?
The alternative implementation, e.g. lodash, is less efficient since it has to handle many corner cases not applicable to Node.js. And lodash detects Node.js and uses
util.is*
functions underneath for efficiency, e.g. https://github.com/lodash/lodash/blob/ddfd9b11a0126db2302cb70ec9973b66baec0975/lodash.js#L452They have great typescript annotations already written and maintained.
What alternatives have you considered?
No response
The text was updated successfully, but these errors were encountered: