Editorial: Use some more abstract operations. #601
Conversation
index.bs
Outdated
| @@ -12636,7 +12635,7 @@ internal method as follows. | |||
| The algorithm operates as follows, with property name |P| and object |O|: | |||
|
|
|||
| 1. If |P| is not a [=supported property name=] of |O|, then return false. | |||
| 1. If |O| has an own property named |P|, then return false. | |||
| 1. If <a abstract-op>HasOwnProperty</a>(|O|, |P|) is <emu-val>true</emu-val>, then return false. | |||
There was a problem hiding this comment.
Unfortunately, I think this is an infinite recursion.
Consider a call to [[GetOwnProperty]] as defined in https://heycam.github.io/webidl/#legacy-platform-object-getownproperty. This calls https://heycam.github.io/webidl/#LegacyPlatformObjectGetOwnProperty which calls into https://heycam.github.io/webidl/#dfn-named-property-visibility for O. The proposed change would then call https://tc39.github.io/ecma262/#sec-hasownproperty on O, which will call right back into O's [[GetOwnProperty]].
What you probably need to do instead is invoke https://tc39.github.io/ecma262/#sec-ordinarygetownproperty directly or something. But note that this uses exactly the same vague "has an own property" (or rather "does not have an own property") language that webidl already has...
index.bs
Outdated
| @@ -12647,7 +12646,8 @@ internal method as follows. | |||
| 1. Let |prototype| be |O|.\[[GetPrototypeOf]](). | |||
| 1. While |prototype| is not null: | |||
| 1. If |prototype| is not a [=named properties object=], | |||
| and |prototype| has an own property named |P|, then return false. | |||
| and <a abstract-op>HasOwnProperty</a>(|prototype|, |P|) is <emu-val>true</emu-val>, | |||
There was a problem hiding this comment.
Is the potentially-observable change here what happens when there's a proxy on the proto chain? What do browsers currently do here?
There was a problem hiding this comment.
They're inconsistent, of course. I'll retake this part after #607 is fixed.
|
The crux here is that in ECMAScript, every object implicitly has a list of properties that’s different from what’s exposed through internal methods, and these references are talking about the implicit properties. See tc39/ecma262#1067 for some more background. |
index.bs
Outdated
| 1. If the property is not configurable, then return <emu-val>false</emu-val>. | ||
| 1. Otherwise, remove the property from |O|. | ||
| 1. Return <emu-val>true</emu-val>. | ||
| 1. Return <a abstract-op>OrdinaryDelete</a>(|O|, |P|). |
There was a problem hiding this comment.
So... UAs are kinda all over the map here (e.g. neither Firefox nor Chrome seems to implement the named property visibility algorithm the way the spec specs it!) but this is observably different from the old spec. That's because OrdinaryDelete calls [[GetOwnProperty]], not OrdinaryGetOwnProperty. This means we'll do the proto walk for the visibility algorithm twice, if UAs were actually implementing the spec. Do we really want that?
There was a problem hiding this comment.
No, I forgot about the distinction between [[GetOwnProperty]] and OrdinaryGetOwnProperty here; will fix.
Notably, this removes some of the vague 'has an own property' language.
|
This should be ready for review again. |
|
At this point it's not clear what this change accomplishes. It replaces text that extracts the one relevant part of OrdinaryGetOwnProperty (step 2, "If O does not have an own property with key P, return undefined.") with a call to the whole of OrdinaryGetOwnProperty, throwing away the result of steps 3-9. This doesn't accomplish the goal implied by #42, of triggering proxy traps. It seems like the above discussion concluded that wasn't desirable? In which case maybe the better resolution is to just close #42 as not desirable, with no changes to the spec text. |
|
I feel we probably should have some clearer explanation that "own properties" only refers to the "real" ones, and doesn't imply a call to the overridden |
|
I guess you could potentially escalate this to TC39 as https://tc39.github.io/ecma262/#sec-object-type only talks about get/set access to these properties, not "has". So perhaps they should define a clearer primitive that can be linked. |
annevk
added
the
do not merge yet
|
Closing this as we use language that's consistent with JavaScript. If JavaScript formalizes own properties more (perhaps in tc39/ecma262#1438) we would follow. |
Notably, this removes the vague 'has an own property' language and fixes #42.
Only in the case of the prototype in the named property visibility algorithm
is this change potentially observable.
Preview | Diff