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

Can we have shorthands for all the methods defined on Object.prototype? #4

Closed Becavalier closed 3 years ago

Becavalier commented 3 years ago

For the symmetry purpose, can we have shorthands for all the methods defined on Object.prototype?

ljharb commented 3 years ago

What others would be useful? Perhaps propertyIsEnumerable, but outside of libraries i haven’t had any call to use that, or any of the other Object.prototype methods.

jamiebuilds commented 3 years ago

Here's a list of everything on Object.prototype:

Object.prototype.__proto__
Object.prototype.constructor
Object.prototype.valueOf()
Object.prototype.toString()
Object.prototype.isPrototypeOf()
Object.prototype.hasOwnProperty()
Object.prototype.__lookupGetter__
Object.prototype.__lookupSetter__
Object.prototype.propertyIsEnumerable()
Object.prototype.__defineGetter__
Object.prototype.__defineSetter__
Object.prototype.toLocaleString()

Ignoring hasOwnProperty() since its already part of this proposal.

I think it's also safe to ignore constructor because that's just new Object

These members are all part of the "Annex B" for browser compatibility and we probably don't want to promote them:

Object.prototype.__proto__
Object.prototype.__lookupGetter__
Object.prototype.__lookupSetter__
Object.prototype.__defineGetter__
Object.prototype.__defineSetter__

These members seem to be specifically designed to be overwritten as necessary so they wouldn't make as much sense on the top-level Object:

Object.prototype.valueOf()
Object.prototype.toString()
Object.prototype.toLocaleString()

That leaves:

Object.prototype.isPrototypeOf()
Object.prototype.propertyIsEnumerable()

I'm honestly not sure what isPrototypeOf is for when we have instanceof but if anything I think that would belong on Reflect not Object

propertyIsEnumerable also seems to be identical to Reflect.getOwnPropertyDescriptor(O, K).enumerable, and I'm not sure it's common enough to warrant something more specific than that.

Object.prototype.hasOwnProperty stands out to me as something that belongs on Object (and not Reflect), is currently inconvenient to do, and developers frequently need it.

Becavalier commented 3 years ago

Make sense, reasonable explanation.

zloirock commented 3 years ago

propertyIsEnumerable also seems to be identical to Reflect.getOwnPropertyDescriptor(O, K).enumerable, and I'm not sure it's common enough to warrant something more specific than that.

Rather !!Reflect.getOwnPropertyDescriptor(O, K)?.enumerable.

A shorthand for Object.prototype.toString could be very useful for classification since Object.prototype.toString.call(foo).slice(8, -1) is a common pattern.

ljharb commented 3 years ago

That pattern was rendered utterly unreliable by ES6, so it’d be a bad idea to encourage its use.

zloirock commented 3 years ago

This pattern is enough correct in the vast majority of cases - duck typing. If someone wanna fake it - he will be able to do it anyway if have the access to the environment.

ljharb commented 3 years ago

“Enough” correct isn’t something that the core language should encourage.

There are plenty of robust ways it can’t be faked; see https://npmjs.com/~inspect-js

zloirock commented 3 years ago

Duck typing is something that should the language encourage - it's good and more than enough for the userland code. Sorry, but most of this list looks for me like unnecessary crutches. More other, as I wrote, almost all of them could be faked.

zloirock commented 3 years ago

For example, your which-builtin-type package which should do the same like the pattern above:

require('which-builtin-type')({ constructor: function RegExp(){} }); // => 'RegExp'

Do you really think that it's better than Object.prototype.toString usage?

ljharb commented 3 years ago

We don’t need to debate it here, but it very much shouldn’t do that - the packages check for internal slots, not the unreliable/useless constructor property.

Yes, i think it’s exceedingly better than ever using Object.prototype.toString, which is why there’s virtually no usage of it in applications i build (even in the dependency tree).

zloirock commented 3 years ago

Yes, I see how good this package checks for internal slots - the good example above. So, I feel sorry for these applications. If you follow a bad practice - please do not impose it on others and, at first, on the language ;-)

ljharb commented 3 years ago

Whether that's a "good" example depends on your use case. { constructor: function RegExp(){} } is not a regex, doesn't inherit from RegExp.prototype (so it doesn't even have RegExp's weird Symbol methods), and won't pass internal slot checks. Anything that describes that object as a RegExp would be what I call a very bad pattern.

Either way, this issue and this repo are not an appropriate place to debate general programming philosophies; suffice to say a brittle Object.prototype.toString-based approach would not achieve consensus.

zloirock commented 3 years ago

Whether that's a "good" example depends on your use case. { constructor: function RegExp(){} } is not a regex, doesn't inherit from RegExp.prototype (so it doesn't even have RegExp's weird Symbol methods), and won't pass internal slot checks. Anything that describes that object as a RegExp would be what I call a very bad pattern.

But why your package shows RegExp for this very bad pattern? -)

It's just a simple example where fails the package which you recommend as the replacement, but the simple toString pattern works perfectly. I could show many other examples, but why? The only way to "break" toString pattern - Symbol.toStringTag - which just adds the possibility of duck typing to this method and is required for proper polyfilling - so it's rather a "fix", not a "break". It's perfect that with this method we could see Set instead of Object for polyfilled Set.

So, you wanna resist duck typing in JS at all, or it the only case because you have many packages for "fixing" it?

ljharb commented 3 years ago

I didn’t realize that was a bug report; certainly I’ll fix it. That’s clearly not something often done in real code, or it’d have been reported sooner.

Duck typing is idiomatic but in no way is it typically robust in its use; i don’t want to encourage brittle patterns even if they’re common.

zloirock commented 3 years ago

You could fix this bug, but checking all possible internal slots on all possible exotic environment objects is doomed to fail. This package is already too bloated, but even if it will be a megabyte, it will fail in more possible cases than toString.

Sometimes in some libraries code we need something like that:

switch (Object.prototype.toString.call(collection).slice(8, -1)) {
  case 'Array': ...;
  case 'Set': ...; // we want that it matches not only global `Set` but also, for example, pure `Set` polyfill instances
  case 'Map': ...; // the same
  default: throw TypeError('Called on unknown collection type');
}

Object.prototype.toString is the best option here and it does not require dozens of kilobytes for this simple detection.

jamiebuilds commented 3 years ago

I think this discussion has gone off in a different direction than the issue's purpose.

I also don't think this is the appropriate proposal to attempt improving runtime type checks. Such a proposal would warrant a much more complete consideration of the language than just promoting methods from Object.prototype to Object, and while I'm supportive of the idea, I'm not going to pursue that here.