util: add util.types.isBoxedPrimitive #22620
Conversation
Checking all boxed primitives individually requires to cross the C++ barrier multiple times besides being more complicated than just a single check.
| } | ||
| } | ||
|
|
||
| // Check boxed primitives. |
There was a problem hiding this comment.
Maybe add some negative results to the test?
There was a problem hiding this comment.
This seems somewhat redundant to me, since this is just a helper combined out of different helpers that are already tested against all other types.
But I'll add negative results as well if you feel strongly about it!
There was a problem hiding this comment.
I think it makes sense to test - but don't feel strongly about it.
|
Maybe name it Edit: It's more consistent with the names of the individual methods (there is no |
|
@targos You'll sometimes see boxed, unboxed, and auto-boxing come up in conversations. It's used to describe what happens when you access a property on a primitive value. For example: "a".trim // auto-boxes primitive "a" to access the property "trim"
"a".a = 1 // box primitive for property assignment then unbox after
"a".a // undefined because the property was assigned to the boxed value which was discardedMDN mentions boxing here , here, and here. That said, |
|
|
That would imply that we’re checking for “object primitives”, i.e. primitives which have something to do with objects. The values that pass the tests are definitely not primitives, though. |
It does seems somewhat more intuitive for me (even if it's not really a primitive it is why we check for it: we want to know if it has something to do with a primitive by encapsulating one). I never heard of an "primitive object" and my head just can't get used to that name. |
I agree that it sounds odd, but I think it’s more accurate than “object primitive”. Tbh, I think |
|
(it was just a suggestion, I'm not blocking anything. Just wanted to make sure alternative names were considered) |
|
I am fine with either |
There was a problem hiding this comment.
isBoxedPrimitive sounds good to me. 
I would also be okay with isObjectWithPrototypeThatIsOneOfThePrimitivePrototypeIntrinsics 😄
|
I dig Update: I should make it clear I'm OK with the existing |
|
Totally scientific research: https://twitter.com/addaleax/status/1036696939365052418 |
|
It's a boxed primitive imo; a "primitive object" doesn't make sense because the two words are mutually exclusive. It's also not a boxed object, it's an object formed by boxing a primitive into an object. (fwiw this is already polyfillable in pure JS; see https://twitter.com/ljharb/status/1036707165002559488) |
|
@jdalton to clarify; that it's already polyfillable is why this is safe to expose directly as a faster and much more friendly helper :-) i'm super on board. |
|
The CI https://ci.nodejs.org/job/node-test-commit/21107/ is green by the way. |
|
Would it be worthwhile to instead implement this in JavaScript? It seems like it's possible to do so, and it's usually faster than an implementation that goes through C++. |
|
@TimothyGu that would require 4 try/catches around calling cached builtin methods, expecting them to throw for a |
|
I just ran a simple benchmark only for boxed numbers and it's ten times faster in C++ (this is mixed input. Using only boxed numbers as input it's two times slower): Note: I optimized tryNumberObject before using it by setting the |
Would you mind explaining why @ljharb? |
|
@benjamingr because the only way to determine if something is a boxed primitive in JS, cross-realm, is to .call a brand-checking prototype method on it, and if it throws, it is one. (
|
|
If no one objects to the current name in the next 24h, I'll go ahead and land it. The twitter poll resulted in 26% for the current name, 26% for |
Checking all boxed primitives individually requires to cross the C++ barrier multiple times besides being more complicated than just a single check. PR-URL: nodejs#22620 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: John-David Dalton <john.david.dalton@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
|
Thanks everyone. Landed in 3209679 🎉 |
Checking all boxed primitives individually requires to cross the C++ barrier multiple times besides being more complicated than just a single check. PR-URL: #22620 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: John-David Dalton <john.david.dalton@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Notable changes:
* fs
* Added a `recursive` option to `fs.mkdir` and `fs.mkdirSync`. If
this option is set to `true`, non-existing parent folders will be
automatically created.
#21875
* Fixed fsPromises.readdir `withFileTypes`.
#22832
* http2
* Added `http2stream.endAfterHeaders` property.
#22843
* module
* Added `module.createRequireFromPath(filename)`. This new method can
be used to create a custom `require` function that will resolve
modules relative to the `filename` path.
#19360
* url
* Added `url.fileURLToPath(url)` and `url.pathToFileURL(path)`. These
methods can be used to correctly convert between `file:` URLs and
absolute paths.
#22506
* util
* Added `util.types.isBoxedPrimitive(value)`.
#22620
* Windows
* The Windows msi installer now provides an option to automatically
install the tools required to build native modules.
#22645
* Added new collaborators:
* boneskull (https://github.com/boneskull) - Christopher Hiller
* The Technical Steering Committee has new members:
* apapirovski (https://github.com/apapirovski) - Anatoli Papirovski
* gabrielschulhof (https://github.com/gabrielschulhof) - Gabriel Schulhof
PR-URL: #22932
Notable changes:
* fs
* Added a `recursive` option to `fs.mkdir` and `fs.mkdirSync`. If
this option is set to `true`, non-existing parent folders will be
automatically created.
#21875
* Fixed fsPromises.readdir `withFileTypes`.
#22832
* http2
* Added `http2stream.endAfterHeaders` property.
#22843
* module
* Added `module.createRequireFromPath(filename)`. This new method can
be used to create a custom `require` function that will resolve
modules relative to the `filename` path.
#19360
* url
* Added `url.fileURLToPath(url)` and `url.pathToFileURL(path)`. These
methods can be used to correctly convert between `file:` URLs and
absolute paths.
#22506
* util
* Added `util.types.isBoxedPrimitive(value)`.
#22620
* Added new collaborators:
* boneskull (https://github.com/boneskull) - Christopher Hiller
* The Technical Steering Committee has new members:
* apapirovski (https://github.com/apapirovski) - Anatoli Papirovski
* gabrielschulhof (https://github.com/gabrielschulhof) - Gabriel Schulhof
PR-URL: #22932
Notable changes:
* fs
* Fixed fsPromises.readdir `withFileTypes`.
#22832
* http2
* Added `http2stream.endAfterHeaders` property.
#22843
* util
* Added `util.types.isBoxedPrimitive(value)`.
#22620
* Added new collaborators:
* boneskull (https://github.com/boneskull) - Christopher Hiller
* The Technical Steering Committee has new members:
* apapirovski (https://github.com/apapirovski) - Anatoli Papirovski
* gabrielschulhof (https://github.com/gabrielschulhof) - Gabriel Schulhof
PR-URL: #22932
Notable changes:
* fs
* Fixed fsPromises.readdir `withFileTypes`.
#22832
* http2
* Added `http2stream.endAfterHeaders` property.
#22843
* util
* Added `util.types.isBoxedPrimitive(value)`.
#22620
* Added new collaborators:
* boneskull (https://github.com/boneskull) - Christopher Hiller
* The Technical Steering Committee has new members:
* apapirovski (https://github.com/apapirovski) - Anatoli Papirovski
* gabrielschulhof (https://github.com/gabrielschulhof) - Gabriel Schulhof
PR-URL: #22932
|
Is this only useful for isolated performance hacks? I'm trying to think of a use-case for it and can't, beyond the kinds of things that go in to |
> var x= new Boolean()
> x
[Boolean: false]
> var y = false
> y
false
> typeof x
'object'
> typeof y
'boolean'
> x === y
falseI believe it's just an aggregate of checks already available in the language. But yeah,
|
|
As a utility lib author this method is exciting because I so run into a need for this kind of check (equality checks, cloning, merging, etc) and would rather defer to an environment builtin when possible. |
|
I did not implement it for performance reasons in the first place. I mainly want to have a utility function that makes sure I know if a variable is a boxed primitive or not. No matter what primitive it is. Otherwise I have a bloated code where I have to go through each boxed primitive manually / write an abstraction for it. Doing it in core seemed the best way of doing it because we do less boundary crossings and it fits well into all the other util.types functions. We have something similar with |
|
OK, I get it, thanks for the insight folks! |
|
For those interested, I ended up making a package for this: https://www.npmjs.com/package/is-boxed-primitive |
Checking all boxed primitives individually requires to cross the C++
barrier multiple times besides being more complicated than just a
single check.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes