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

Naming bikeshed #3

Closed
zloirock opened this issue Apr 6, 2021 · 24 comments
Closed

Naming bikeshed #3

zloirock opened this issue Apr 6, 2021 · 24 comments

Comments

@zloirock
Copy link

zloirock commented Apr 6, 2021

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.

@zloirock zloirock changed the title Rename to hasOwn Rename to Object.hasOwn Apr 6, 2021
@jamiebuilds
Copy link
Member

jamiebuilds commented Apr 6, 2021

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
Copy link
Author

zloirock commented Apr 6, 2021

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
Copy link
Member

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
Copy link
Author

zloirock commented Apr 6, 2021

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

ljharb commented Apr 6, 2021

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
Copy link
Member

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
Copy link
Author

zloirock commented Apr 7, 2021

@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
Copy link
Member

jamiebuilds commented Apr 7, 2021

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
Copy link

bakkot commented Apr 7, 2021

@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
Copy link

NemoStein commented Apr 16, 2021

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
Copy link
Member

ljharb commented Apr 16, 2021

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

@jamiebuilds
Copy link
Member

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
Copy link

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
Copy link
Member

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

@jamiebuilds jamiebuilds added bug and removed concern labels Apr 19, 2021
@jamiebuilds jamiebuilds changed the title Rename to Object.hasOwn Naming bikeshed Apr 20, 2021
@jamiebuilds jamiebuilds pinned this issue Apr 20, 2021
@jamiebuilds
Copy link
Member

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.

TimvdLippe added a commit to TimvdLippe/proposal-accessible-object-hasownproperty that referenced this issue Apr 22, 2021
These probably were meant to be `Object.hasOwn` per tc39#3 (comment)
@TimvdLippe TimvdLippe mentioned this issue Apr 22, 2021
jamiebuilds pushed a commit that referenced this issue Apr 22, 2021
These probably were meant to be `Object.hasOwn` per #3 (comment)
@Andrew-Cottrell
Copy link

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
Copy link
Member

ljharb commented May 17, 2021

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

@jamiebuilds
Copy link
Member

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

@jamiebuilds jamiebuilds unpinned this issue May 25, 2021
@deecewan
Copy link

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
Copy link
Member

ljharb commented Jul 14, 2021

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
Copy link

awesome, thanks for that 😄

@jridgewell
Copy link
Member

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

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

8 participants