archives

« Bugzilla Issues Index

#3206 — Unscopables should use HasProperty and not Get


http://people.mozilla.org/~jorendorff/es6-draft.html#sec-object-environment-records-hasbinding-n

Step 9.a should be HasBinding and not Get. We do not care about getter side effects here and we do not care about the value.


The reason for doing a [[Get]] is so a subclass can over-ride a unscopables item of a super class:

SubArray.prototype[Symbol.unscopables] = {
__proto__: Array.prototype[Symbol.unscopables],
values: undefined, //remove 'values' from unscopable set of SubArray
};


(In reply to Allen Wirfs-Brock from comment #1)
> The reason for doing a [[Get]] is so a subclass can over-ride a unscopables
> item of a super class:
>
> SubArray.prototype[Symbol.unscopables] = {
> __proto__: Array.prototype[Symbol.unscopables],
> values: undefined, //remove 'values' from unscopable set of SubArray
> };

What is the use case for that?

If we are doing Get, should we not do a ToBoolean on the value?


I think it is good to have a general uniform rule for interpreting properties whose sole purpose of is to provide a yes/no answer. There are various possibilities:

1. HasProperty(obj, prop)
2. Get(obj, prop) !== undefined
3. ToBoolean(Get(obj, prop))
4. etc.

Option 1 is not good (as a general rule), because of impossibility to override a "yes" answer found on the prototype. Option 3 is imho the best one, because it is more intuitive to have `false` meaning "no".

Currently, Option 3 is chosen for @@isConcatSpreadable and @@isRegExp, and a variant of it for @@hasInstance -- namely (oversimplifying a bit):
ToBoolean(Invoke(obj, @@hasInstance, (C))

I think that ToBoolean(Get(...)) should also be chosen here for uniformity, unless there is a good reason against it.


After thinking more about this, I prefer using `undefined` instead of ToBoolean() === false. The main reason is that there are less chances of unexpected side effects.


(In reply to Erik Arvidsson from comment #4)
> After thinking more about this, I prefer using `undefined` instead of
> ToBoolean() === false. The main reason is that there are less chances of
> unexpected side effects.

I don't understand your point, since ToBoolean() itself is side-effect free (recall that all objects are truthy without computation). In fact, regarding potential side-effects, one must choose between those of HasProperty() and those of Get(). Or I missed something?


I did a quick review over the spec. and the most common way handle situations like this is treat both undefined and null as meaning not there.

Basically, undefined covers the non-existent property case and null is a nice value some people like to use (rather than an explicit undefined) as a "not here" marker.

so I'm changed step 9.c. to:
c. If blocked is neither undefined nor null, return false.

which is the usual formulation we use for such tests

fixed in rev32 editor's draft


I'm not happy with this. I think we should either stick to just *undefined* or do ToBoolean and compare the result with false. Having something in between just leads to more cases to remember.


(In reply to Erik Arvidsson from comment #7)
> I'm not happy with this. I think we should either stick to just *undefined*
> or do ToBoolean and compare the result with false. Having something in
> between just leads to more cases to remember.

I've reconsidered, and think it should be ToBoolean.

The reason is that when we defined an unscopables array we use 'true' as the value of the properties. While we don't explicitly say do, that strongly suggests 'false' would mean the opposite of 'true'. Or, in otherwords we expect the values of these properties to be treated as ToBoolean values.

So, step 9.c becomes:

c. If ToBoolean(blocked) is true, return false.


fixed in rev32 draft