The .get and .has methods on weak collections return undefined when a key is not an object. That is inconsistent with .set, which throws.
I'm not sure if this is a recent change (the V8 implementation always throws, and I assume we were following the spec draft at the time). Either way, it doesn't make sense to consider non-objects valid keys in some operations but not others.
Related Mozilla issue (Resolved fixed): https://bugzilla.mozilla.org/show_bug.cgi?id=1127827
Related Chromium issue: https://code.google.com/p/chromium/issues/detail?id=460083
For reference, the behavior about handling of non-object values in WeakMap has been updated in rev20 draft, Oct. 28, 2013:
https://bugs.ecmascript.org/show_bug.cgi?id=1938
http://wiki.ecmascript.org/doku.php?id=harmony:specification_drafts#october_28_2013_draft_rev_20
Thanks for the reference, I obviously missed that change. Sorry or being late on this.
I agree with the implication requested in comment #2 on that bug. However, Mark's suggestion "fixed" it the wrong way. Having all methods throw consistently on non-object keys would achieve the same implication, and in addition, provide a consistent behaviour for invalid keys.
That is, in addition to Andre's implication, the following equivalences should also hold:
m.set(k,v) throws <=> m.get(k) throws <=> m.has(k) throws <=> m.delete(k) throws
and similarly for sets.
Hi Andreas, we seem to disagree about which inconsistency is greater. The way I see it, WeakMap.set must satisfy only a subset of Map.set's contract, by rejecting all non-object keys. But, given that it does so, then the mapping cannot contain any non-object keys.
WeakMap.has, .get, and .delete can then fulfill the full Map.has, .get, and .delete contract, as they would on a map whose mapping did not contain any non-object keys -- whether or not they could.
Put another way, .set has the specialized job of admission control. .has, .get, and .delete need only deal with what was admitted.
----
We can also look at it from the side of multiplying special cases in the clients of these abstractions. A client of a mapping abstraction that might be given a Map or a WeakMap, and that does a .set, must be prepared for .set(primValue) to throw, or must avoid that case. Likewise, it must not use any of the Map api that is absent from WeakMap. But a client who is only given a mapping that is populated by others, and only does a .get, .has, and .delete on it should not need to care about the difference, since both mapping abstractions can satisfy the same contract for these.
(In reply to Mark Miller from comment #4)
> Hi Andreas, we seem to disagree about which inconsistency is greater. The
> way I see it, WeakMap.set must satisfy only a subset of Map.set's contract,
> by rejecting all non-object keys. But, given that it does so, then the
> mapping cannot contain any non-object keys.
>
> WeakMap.has, .get, and .delete can then fulfill the full Map.has, .get, and
> .delete contract, as they would on a map whose mapping did not contain any
> non-object keys -- whether or not they could.
>
> Put another way, .set has the specialized job of admission control. .has,
> .get, and .delete need only deal with what was admitted.
>
> ----
>
> We can also look at it from the side of multiplying special cases in the
> clients of these abstractions. A client of a mapping abstraction that might
> be given a Map or a WeakMap, and that does a .set, must be prepared for
> .set(primValue) to throw, or must avoid that case. Likewise, it must not use
> any of the Map api that is absent from WeakMap. But a client who is only
> given a mapping that is populated by others, and only does a .get, .has, and
> .delete on it should not need to care about the difference, since both
> mapping abstractions can satisfy the same contract for these.
I see. This is presuming that weak collections are a useful drop-in replacement for a general ones. Not sure they ever are, though. Their contracts are far more restrictive, and consequently, they're _not_ in a semantic subclass relation. I'm not convinced it's particularly useful to maintain that illusion for a tiny subset of their interface.
A context that would work correctly with both weak and non-weak collections but falls over with the semantics I'm proposing would have rather rare properties: (1) it doesn't iterate the collection, (2) it wants to check the collection for keys that are not objects, but (3) it is guaranteed to never store non-objects.
Weighing this against an internally consistent treatment of key types in weak collections, I find the latter more relevant.
Another vote in support for type checking in get/has.
Lets do an analogy. If I implement a StringMap where the keys have to be strings, I could let `get` take any type and always return undefined if not a string. But I find it more useful to throw in that case to show that the caller passed in something non meaningful.
(In reply to Erik Arvidsson from comment #6)
> Another vote in support for type checking in get/has.
>
> Lets do an analogy. If I implement a StringMap where the keys have to be
> strings, I could let `get` take any type and always return undefined if not
> a string. But I find it more useful to throw in that case to show that the
> caller passed in something non meaningful.
Interesting. When I first started reading your analogy, I expected exactly the opposite conclusion.
Perhaps "delete" is confusing the picture. If we omit it from the discussion for a moment, then my answer to Andreas'
> A context that would work correctly with both weak and non-weak collections
> but falls over with the semantics I'm proposing would have rather rare
> properties:
> (1) it doesn't iterate the collection,
> (2) it wants to check the collection for keys that are not objects, but
> (3) it is guaranteed to never store non-objects.
Is that it is frequent to separate updating contexts from pure querying contexts. A pure querying context is guaranteed never to store non-objects because it never stores anything. That is up to the producer of the mappings it consumes. To a consumer, it hardly matters whether the producer did not store an X vs could not store an X. Precedent:
> "abc".indexOf({})
-1
Would you rather have that throw?
Had we specified that deleting a key that doesn't exist throws, then "delete" would not be such an oddball. OTOH, if we consider consuming consumers (e.g. linear consumers) in addition to side-effect-free consumers, then considering "delete" to be part of the more lenient consuming type makes some sense.
(In reply to Mark Miller from comment #7)
> > "abc".indexOf({})
> -1
>
> Would you rather have that throw?
If there was a do-over? Yes.
(In reply to Mark Miller from comment #7)
> (In reply to Erik Arvidsson from comment #6)
> > Another vote in support for type checking in get/has.
> >
> > Lets do an analogy. If I implement a StringMap where the keys have to be
> > strings, I could let `get` take any type and always return undefined if not
> > a string. But I find it more useful to throw in that case to show that the
> > caller passed in something non meaningful.
>
> Interesting. When I first started reading your analogy, I expected exactly
> the opposite conclusion.
>
> Perhaps "delete" is confusing the picture. If we omit it from the discussion
> for a moment, then my answer to Andreas'
>
> > A context that would work correctly with both weak and non-weak collections
> > but falls over with the semantics I'm proposing would have rather rare
> > properties:
> > (1) it doesn't iterate the collection,
> > (2) it wants to check the collection for keys that are not objects, but
> > (3) it is guaranteed to never store non-objects.
>
> Is that it is frequent to separate updating contexts from pure querying
> contexts. A pure querying context is guaranteed never to store non-objects
> because it never stores anything. That is up to the producer of the mappings
> it consumes.
Yes, but that is ignoring the other two conditions. It would have to be a pure consumer that does not iterate. And one that somehow expects to be able to ask for non-object keys in a context where there can't possibly be any. I'm too uncreative to imagine a scenario for that.
> To a consumer, it hardly matters whether the producer did not
> store an X vs could not store an X. Precedent:
>
> > "abc".indexOf({})
> -1
>
> Would you rather have that throw?
Most definitely. The likelihood of this not being the symptom of a bug seems close to 0.
so, we either need to resolve this as "works as intended" or decided to make a change.
My default response is likely to be the former as Mark is the champion of this feature and the spec. reflects his position and also the result of past discussions on this matter.
However, I will point out that throwing an exception is behavior that could be relaxed in the future if it proves to be a bad choice, while there isn't really any backwards compatible way to reverse the decision if we keep the no exception alternative.
(In reply to Allen Wirfs-Brock from comment #10)
> so, we either need to resolve this as "works as intended" or decided to make
> a change.
>
> My default response is likely to be the former as Mark is the champion of
> this feature and the spec. reflects his position and also the result of past
> discussions on this matter.
>
> However, I will point out that throwing an exception is behavior that could
> be relaxed in the future if it proves to be a bad choice, while there isn't
> really any backwards compatible way to reverse the decision if we keep the
> no exception alternative.
a) I agree with that last point. When it is not clear what the right decision is, if there's a way to safely and compatibly postpone the decision, I've generally been in favor of that.
b) I also agree that it is not clear in this case what the right decision is. My intuition remains that the current behavior is better. But my rationale for that is not strong enough to be confident I can't be convinced otherwise.
c) I do understand that, in terms of committee dynamics, consenting to this change now makes it unlikely that I'll get it the "right" way in the future. Today, if we don't get consensus to change, then the "right" status quo wins. If we make it throw in ES6, then if we don't get consensus to change later, the "wrong" status quo wins. However, given #a and #b, I think I need to concede to this change.
One more time though, here's a further explanation of my intuition here:
In a statically typed world, it makes sense to see abstractions as partial functions of the set of all possible inputs -- the type system prevents most inputs from even being tried at runtime. In a dynamic system, for many purposes we still think of abstractions as partial, but the situation is more mixed. If we consider throw to be among the responses, then abstractions are total -- they may be given any first class value at runtime. In response to, let's say, a boolean query, if the answer to the question being asked is true, I expect the query to respond "true". If the answer to the question being asked false, I expect the query to respond "false". If the response is throw, I expect this indicates that the query is ill formed or meaningless, or at least something for which neither "true" nor "false" is a logically sound response to the query's meaning.
map.has(88)
asks whether the map has 88 as a key. If it does not, it should tell me that. Throwing is basically saying: "I refuse to tell you whether I have 88 as a key, even though I know, because I don't think you should have asked."
Good API design for a dynamic language is different than good API for a dynamic language.
Have I convinced anyone?
Allen, independent of the historical, scheduling, and committee issues, as a dynamic language design person, I am curious what you think. Care to weigh in?
> Good API design for a dynamic language is different than good API for a
> dynamic language.
Oops. Not what I meant ;).
(In reply to Mark Miller from comment #11)
>
>
> Allen, independent of the historical, scheduling, and committee issues, as a
> dynamic language design person, I am curious what you think. Care to weigh
> in?
I'm fine with the current behavior. My preference is to allow programs to continue making forward progress until it reaches a point where that is impossible. If, upon the basis of map.has returning false, a program tries to do something that is impossible to do such as set a non-object key in a WeakMap, then the point of the set is probably where the exception should occur.
I plan closing this as WONTFIX (really "Works as intended") unless somebody intends to bring this up at the March TC39 meeting as a ES6 approval blocker.