Skip to content
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

The usage of the term "key" is ambiguous in relation to object properties #1110

Closed
ghost opened this issue Feb 20, 2018 · 9 comments
Closed

Comments

@ghost
Copy link

ghost commented Feb 20, 2018

It seems there are (at least) two definitions of the term "key" within the specification that are not differentiated/explicitly defined.

The most common definition is summarised as: an object property identifier of type string or symbol.
For discussion I'll call this pid (for property ID).

The other definition is more traditional: a string that serves to identify an object in a hash table/map.
For discussion I'll call this hid (for hash table ID).

Where this generates confusion relates to the common usage of an object as a key/value store. In that use case a key is a hid. In all other use cases a key is a pid. This ambiguity is propagated throughout the language itself, with some functions on Object using one definition and others using the other without remark.

Some examples of Object functions that use pid (those that include symbol keys):

  • assign: Assigns all properties including symbols
  • create: Second param defines properties, including symbol properties
  • defineProperty and defineProperties: self-explanatory
  • seal: also seals symbol properties

Some examples of Object functions that use hid:

  • keys: Only returns keys that are of type string
  • entries: Only returns key-value pairs where the keys are strings
  • values: Only returns values whose keys are strings

The above lists cover a good number of functions but they're not exhaustive.

Note that the documentation for those three functions on MDN states that all enumerable properties are included, but this is not the case. It's necessary to read the spec to see that these functions only include string keys currently (which is unrealistic to expect from everyone, however desirable).

For convenience this MDN page links to all functions on Object: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object

Aside from the MDN documentation this has also caused confusion for the Typescript contributors (which is how I became aware of the problem):

Difficulty deciding on the correct type of the return value of Object.keys: microsoft/TypeScript#12253 (comment)

Ambiguity causing the TS keyof keyword to be implemented using the hid definition of a key rather than the pid definition. Any change to which would now cause breaking changes to all Typescript programs: microsoft/TypeScript#20721 and microsoft/TypeScript#21983

In short it seems that the dual use of Object as both a hash table and as the base "object" type has generated (and will likely continue to generate) confusion because it's so easy to overlook.

For specifics with references to a few relevant parts of the ECMA specification itself see my comment on this issue here: microsoft/TypeScript#21983 (comment)

One potential solution would be to support overloading of the indexing operator in classes, add this to Map and change the hid functions of Object to pid functions. However that would not be backwards compatible so I'm not really sure how this could be resolved? We may be stuck with it indefinitely but at least the spec can be updated to caution on the ambiguity?

@ljharb
Copy link
Member

ljharb commented Feb 21, 2018

The MDN documentation should be changed to qualfify that those methods return all enumerable string properties, at the least.

@bterlson
Copy link
Member

bterlson commented Feb 21, 2018

6.1.7 seems pretty clear to me - strings and symbols are property keys. Do you have specific suggestions in mind?

@ghost
Copy link
Author

ghost commented Feb 21, 2018

I think that EnumerableOwnProperties should be renamed to EnumerableOwnNamedProperties or something similar. https://tc39.github.io/ecma262/#sec-enumerableownproperties

Object should be defined as a union of two "transient" (for lack of a better word) types: BaseObject and DictionaryObject (please do suggest better names if you have any ideas, these seem awful!) Those two types should be described in a way such that readers immediately understand that BaseObject contains those properties which relate to the domain of a core language object, whilst DictionaryObject should be described as containing helper properties to support the common use of an object as a dictionary.

In that way readers will know not to apply anything from DictionaryObject to their reasoning about objects in general.

I've used property for both "fields" and "methods" as that seems to be the terminology used in the spec.

@domenic
Copy link
Member

domenic commented Feb 21, 2018

While I think these distinctions might make sense for a type system, I don't think they make sense for a language specification.

@bterlson
Copy link
Member

EnumerableOwnPropertyNames is a more appropriate name for EnumerableOwnProperties, and it seems like a reasonable change. But I agree with @domenic that defining objects as a union isn't a net win for spec clarity.

@ghost
Copy link
Author

ghost commented Feb 21, 2018

Edit: I drafted this reply before sleep and hadn't seen @bterlson's comment before I sent it. Trying to think of other solutions if everyone is opposed to the union idea (although perhaps the idea below is more agreeable with the OOP phrasing?)

It still has a type system, it's just described in natural language and pseudo-code instead of being described in terms of itself. E.g., like mathematics.

I think the broader issue is that we have two different types masquerading as one. We have Object and Dictionary. We need to strongly characterise the differences between the two in the spec and then redefine what is currently "Object" under a new name, such as Prototype or something.

interface Dictionary { entries() {}  keys {}  values {} }
interface Object { ...everything else... }
class Prototype implements Object, Dictionary {}

Something like that. Where the "interfaces" Dictionary and Object don't exist outside of the specification, and Object in the language itself always refers to Prototype. It's ugly but the current Object violates the separation of concerns principle which is the ultimate source of the confusion.

Maybe it would be better as:

interface DictionaryObject { entries, keys, values... }
interface CoreObject { ...everything else... }
class Object implements CoreObject, DictionaryObject {}

Any thoughts?

@bterlson
Copy link
Member

I really appreciate any thinking around how to make the spec more clear and less error-prone, but I am not convinced that splitting object semantics into a union will help matters. To me it seems harder to understand.

@rwaldron
Copy link
Contributor

@ljharb https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object$compare?to=1361313&from=1340351 👍

@bterlson
Copy link
Member

I'm going to close this. @ogwh if you would like to discuss further changes, feel free to re-open or file a more targeted issue. Thanks!!

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

No branches or pull requests

4 participants