Skip to content
This repository has been archived by the owner on Jan 25, 2022. It is now read-only.

Tracking issue for in-the-field feedback #18

Open
jamiebuilds opened this issue Jun 8, 2021 · 9 comments
Open

Tracking issue for in-the-field feedback #18

jamiebuilds opened this issue Jun 8, 2021 · 9 comments

Comments

@jamiebuilds
Copy link
Member

jamiebuilds commented Jun 8, 2021

Using this issue as a central place to collect community feedback for in-the-field experience, positive or negative.

@robertt
Copy link

robertt commented Jun 8, 2021

Wasn't too big of a change, but the codemod replaced ~80 instances and everything seems to be working fine! Will continue to test and I'll come back if I notice anything 🎉

@lehni
Copy link

lehni commented Jun 17, 2021

I always thought it was strange, naming consistency wise, that there is Object.keys() and Object.prototypes.hasOwnProperty(). In an ideal world, for me this would have been hasOwnKey(), and now here Object.hasOwnKey() too?

@ljharb
Copy link
Member

ljharb commented Jun 17, 2021

@lehni "key" means "string property"; Object.prototype.hasOwnProperty and Object.hasOwn check both string and Symbol properties.

@jamiebuilds
Copy link
Member Author

jamiebuilds commented Jun 17, 2021

@ljharb I wouldn't say that distinction is very strongly held, Reflect.ownKeys() returns symbols (although I would agree that that is a unfortunately named API)

The bigger argument is that most methods are not named with their "subject" -- its array.includes() not array.includesItem(), there are lots of examples especially with more recent APIs.

The point is that we couldn't use hasOwnProperty as the name for backwards compatibility reasons, and the "key" or "property" portion is largely unnecessary -- again its map.has() not map.hasKey()

@ljharb
Copy link
Member

ljharb commented Jun 17, 2021

Good point about Reflect.ownKeys, but I think it'd be a hard argument to make that Object.keys wasn't the thing setting the precedent :-) plus, while I think Reflect.hasOwnKey would be consistent, Object.hasOwnKey checking symbols too might be inconsistent.

@jonathantneal
Copy link

Hi, @ljharb! The internet brought me here. So hi! And, sorry?

"key" means "string property"

Have I been getting this wrong? Or do you mean "key" means "string property" only in Object.keys()? (And maybe implicitly in Object.values & Object.entries, too. 😬)

Until now, my understanding was that Object.keys should have been Object.names, and that I was to think about the terms like this:

term expectation
has "returns whether the property is available"
hasOwn "returns whether the property is directly available"
hasName "returns whether the string property is available"
hasOwnName "returns whether the string property is directly available"
hasSymbol "returns whether the symbol property is available"
hasOwnSymbol "returns whether the symbol property is directly available"

And that ☝️ understanding is why a proposal for Object.hasOwn would appropriately mirror Object.prototype.hasOwnProperty.

Otherwise, if "key" should mean "string", that would mean Reflect was not a helpful precedent for Object. I might then make a logical leap and expect Object.hasOwn to not work like Object.prototype.hasOwnProperty, but rather to work like Object.keys, Object.values, and Object.entries, where only string properties are used.

Mind helping me out by clarifying which terms mean what? It might help me unpack all this other mental baggage. 😅

@ljharb
Copy link
Member

ljharb commented Jun 24, 2021

@jonathantneal the reality is unfortunately that there isn't a consistently clear usage of terms in the language :-( Object.keys is own enumerable strings, Reflect.ownKeys is own strings or symbols, Object.getOwnPropertyNames is own strings, Object.getOwnPropertySymbols is own symbols, the spec calls a "string or symbol" a "Property Key", etc.

In general, I don't think Reflect should be considered a precedent for anything, since it's a weird niche Proxy trap thing, and the spec itself is a pretty weak precedent for anything in the presence of any userland precedent.

That leaves us with Object.keys vs Object.getOwnPropertyNames, but I don't think anyone actually thinks a "key" means an enumerable string and a "name" means a non-enumerable string, so we're still kind of stuck.

Using "hasOwn" and leaving the predicate implicit seems like it sidesteps all this confusion, since there's no possible implication that it's just a string or just a symbol.

@jamiebuilds
Copy link
Member Author

Object.hasOwn() has been in Firefox Nightly for the last month and they say there haven't been any reported issues: https://bugzilla.mozilla.org/show_bug.cgi?id=1711872#c8

@robatwilliams
Copy link

For anyone considering the codemod from lodash, the notable difference is that _.has() is null-safe whereas this isn't (which is a good thing IMO).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants