tc39 / proposal-accessible-object-hasownproperty

Object.hasOwn() proposal for ECMAScript
https://tc39.es/proposal-accessible-object-hasownproperty/
MIT License
134 stars 8 forks source link

Naming bikeshed #3

Closed zloirock closed 3 years ago

zloirock commented 3 years ago

I perfectly understand that has is more frequent name than hasOwn for this shortcut, but we have Reflect.has which works like in operator. The rest Reflect methods work similarly to Object methods - we should avoid this inconsistency. Also, I think, the method name should point to check own properties.


Edit from @jamiebuilds:

This proposal was presented to TC39 on April 20, 2021, where it went forward to Stage 2 but the name was left in question. This issue should be used for discussing the method name further.

jamiebuilds commented 3 years ago

I had noted this in a previous version of the proposal, but there also exists Map/Set/FormData/etc#has which all have slightly different behaviors which don't seem to be confusing. And there already exists a parallel in Object:

Object.entries(object) // "own" enumerable properties
Map.prototype.entries()
Set.prototype.entries()
FormData.prototype.entries()
// etc.

If Object.entries() had been Object.entriesOwn() or Object.ownEntries() I think there'd be a stronger argument that it should be Object.hasOwn() but as it is it seems like it'd be more surprising to name it hasOwn.

zloirock commented 3 years ago

It's absolutely different data structures. Object.entries unlike Map.prototype.entries is static and returns an array, not an iterator so it's a strange parallel. There are many over methods with own, for example, Reflect.ownKeys, which is the main method of work with object keys since supports all possible own object keys. Object.has(Own)? should also support symbols and non-enumerable keys, so the parallel is more obvious here.

jamiebuilds commented 3 years ago

I still think the parallel is stronger between data structures like Map and Object, yes they return different types but that is also consistent:

Object.entries() // array
Map.prototype.entries() // iterator

Object.keys() // array
Map.prototype.keys() // iterator

Reflect is a very unique namespace in JS (to quote @ljharb in #2):

Reflect's purpose is indeed only to contain, 1:1, a method for each Proxy trap.

I think for the use case of "objects-as-dictionaries" people would sooner relate them to other data structures like maps and sets long before they would relate them to Reflect (which I would be surprised if your average JavaScript developer was even aware of let alone has used it before) which has less to do with "data structures" and operates as a language interface.

zloirock commented 3 years ago

Ok, Object.getOwnPropertyNames, Object.getOwnPropertySymbols which works with symbols and non-enumerable keys with which should work Object.has(Own)? - methods like Object.keys should not handle them.

jamiebuilds commented 3 years ago

Maybe someone else can comment on the mental model that should be used for symbols. My understanding of how they get modeled into the language is that they should always be observable when you have access to them already, and (generally) not be observable when you don't have access to them. I thought Object.getOwnPropertySymbols() was more of an escape-hatch mechanism.

Using that mental model, it makes sense that enumerating methods like entries() and keys() would omit symbols while methods like has(p) would include them.

For non-enumerable object properties, I think this is just covering the inherent differences between Object and Map, Maps don't have a concept of "enumerability" in its members, but objects do. Similarly, Sets don't have a concept of "key->value" and yet support .keys()/.values()/.entries() to mirror Maps.

jridgewell commented 3 years ago

Object.keys and Object.getOwnPropertyNames are ES5, so it existed before Symbols. We couldn't add symbols to the return value in ES6 without it being a breaking change. Object.values and entries follows that precedent.

Back on the main point, you're very likely to get criticism in committee that Reflect.has already exists and does something different. The names in Reflect that have equivalent Object methods behave the same.

ljharb commented 3 years ago

The names on Reflect imo already lack consistent consistency: Reflect.ownKeys vs Object.keys, Reflect.deleteProperty(o, k) vs delete o[k], Reflect.get(o, k) vs o[k], Reflect.set(o, k, v) vs o[k] = v, Reflect.apply vs Function.prototype.apply, Reflect.construct vs new - out of the 13 methods, 6 don't match, 6 do, and the last is has.

I'm not sure there's a strong naming argument to be made whatsoever based on Reflect.

jamiebuilds commented 3 years ago

Oh to be clear, I'm just trying to make the point that there is a tradeoff here between mirroring other javascript data structures vs differentiating the method from Reflect.

I think an important difference here is that data structures like Object, Map, Set, etc are a daily part of using modern JavaScript, where using something like Reflect is extremely rare.

Searching through publicly available code, it's difficult to find real-world examples of developers using Reflect.has() (most of the results that come up are ES shims, vendoring of spidermonkey tests or test262, etc).

Being so rare, it seems unlikely that any JS engineers would have confusion around Reflect.has()

zloirock commented 3 years ago

@ljharb sorry, but it's a strange argumentation and it's not applicable here. We talk only about Object and Reflect and ALL their intersecting methods are consistent. It's not applicable to compare Reflect.ownKeys and Object.keys since they do different things.

jamiebuilds commented 3 years ago

The several intersecting methods of Object and Reflect are largely a coincidence of the order various features were added to the language, not something that was actively sought. The point of Reflect is not to be a collection of other globals' methods.

For example, Object.defineProperty() predates Reflect and was added to object because that was the most sensible choice at the time. Then Reflect was added because there needed to be a better place for those types of methods. JavaScript can't make breaking changes, so now we have both Object.defineProperty() and Reflect.defineProperty().

But if the argument is about intersecting methods on different objects, we should be looking at Map, Set, and others.

Object, Map, Set, FormData, URLSearchParams and an increasing number of JavaScript globals have a common interface. Many of their methods intentionally intersect with one another. Sometimes going out of their way to do so (For example, Set.prototype.keys() is arguably nonsensical, but it's worthwhile to have a shared interface).

If you start listing them all out, there is a pretty clear pattern here:

Object.keys()
Map.prototype.keys()
Set.prototype.keys()
FormData.prototype.keys()
URLSearchParams.prototype.keys()

Object.values()
Map.prototype.values()
Set.prototype.values()
FormData.prototype.values()
URLSearchParams.prototype.values()

Object.entries()
Map.prototype.entries()
Set.prototype.entries()
FormData.prototype.entries()
URLSearchParams.prototype.entries()

// Object.has() -- missing!
Map.prototype.has()
Set.prototype.has()
FormData.prototype.has()
URLSearchParams.prototype.has()

This isn't a coincidence, it's not the result of different features being added at different times. It's the result of intentional design choices. Design choices that are worth preserving between the data structures that JavaScript developers use everyday.

bakkot commented 3 years ago

@jamiebuilds I came into this thinking hasOwn would make more sense, but that's a pretty convincing table. If you are putting together a presentation for TC39, you should include that table.

NemoStein commented 3 years ago

I'd like to propose Object.owns instead of Object.has or Object.hasOwn, for the simple fact that own is already a possessive verb (e.g.: "I own a car."). It keeps the intent clear, is short and doesn't collides with Reflect or Map/Set (among others).

BTW, I'd say that Object.has should mirror Reflect.has, since the name implies that the object has some property, without stating that it owns the property. On the other hand, Object.owns implies (in my opinion) that the object owns (or not) the property.

Edit: own should be owns, the same way it's has not have

ljharb commented 3 years ago

"own" is a verb and does not imply that to me.

jamiebuilds commented 3 years ago

Using a completely different name implies that its a completely separate type of operation. But in reality they are the same operation, just for different data types.

Look at Map.prototype.has() vs Set.prototype.has(). In terms of their specific implementation they are not the same (Map vs Set). You could make the argument that they should have been named Map.prototype.hasKey() or Set.prototype.hasItem(). But they aren't named that way because we aren't talking about their specific implementation, we're talking about has as an abstract operation that can be translated from one data type to another.

In that sense, Object.has() is the same operation as Map.prototype.has() or Set.prototype.has(). It's the same operation being translated for the data type you are currently talking about.

"objects-as-dictionaries" is a well established pattern in JavaScript that already has APIs built around it. APIs that are built to mirror Map or Set.

NemoStein commented 3 years ago

In that sense, Object.has() is the same operation as Map.prototype.has() or Set.prototype.has(). It's the same operation being translated for the data type you are currently talking about.

Completely agree, and that's why I think Object.has would likely imply that the object have this property, independently if it's an owned property or not. On the other hand, the argument that Object.entries returns owned properties only (which I personally think it's not what a developer would expect and already saw multiple cases where it caused confusion) makes completely sense when put together with other datatypes.

jamiebuilds commented 3 years ago

Yeah Object.entries/keys/values all operate on own properties, so it doesn't make sense to deviate now

jamiebuilds commented 3 years ago

Stage 2 Update

This proposal was promoted to Stage 2 on April 20, 2021. The full agenda notes have not been published yet. But the committee agreed to move forward with the name Object.hasOwn() and to have further discussions about the name before Stage 3. This issue can be used for further discussion about the name.

Andrew-Cottrell commented 3 years ago

If the function would always return false for symbol property names — unlike Object.prototype.hasOwnProperty — then I suggest the names Object.hasKey and Object.includesKey as alternatives to Object.hasOwn. The reason being: the function would be equivalent to Object.keys(object).includes(propName) rather than Object.prototype.hasOwnProperty.call(object, propName).

ljharb commented 3 years ago

It wouldn’t; it would check either string or symbol keys, just like hasOwnProperty.

jamiebuilds commented 3 years ago

This proposal has reached Stage 3 so the name Object.hasOwn is now locked in

deecewan commented 3 years ago

I don't quite follow. The README links here for "Why hasOwn?" It seems like there's a bunch of justification for Object.has....and then just "The committee moved forward with Object.hasOwn".

Was there reasoning for hasOwn over has?

ljharb commented 3 years ago

There was a bunch of justification not to use has, much of which is in this thread.

You can read the notes here: https://github.com/tc39/notes/blob/25ea12a2f9c0f41588119e94aa01997299adb7f5/meetings/2021-05/may-25.md#accessible-objectprototypehasownproperty-for-stage-3

deecewan commented 3 years ago

awesome, thanks for that 😄

jridgewell commented 3 years ago

Naming was brought up in https://github.com/tc39/notes/blob/25ea12a2f9c0f41588119e94aa01997299adb7f5/meetings/2021-04/apr-20.md#objecthas-for-stage-1