Array.prototype.contains solutions
From: es-discuss [mailto:es-discuss-bounces at mozilla.org] On Behalf Of Domenic Denicola
Or you could attack the problem at the for-in level
This half-sentence was a leftover from an earlier pass; please ignore it.
A more general solution seems like a good idea. Renaming doesn't really solve the deeper issue. Extending native prototypes is a JavaScript "thing" and something that will most likely continue continue.
Ember adds methods to Array.prototype and Function.prototype by default: emberjs.com/api/classes/Ember.Array.html#method_contains (btw their Array#contains is not ES7 spec compliant either)
then there's the oldie Prototype.js: api.prototypejs.org
and alternatives like Sugar.js: sugarjs.com/api
that's a lot to watch out for.
JDD
On 9/30/14, 8:12 PM, John-David Dalton wrote:
Extending native prototypes is a JavaScript "thing" and something that will most likely continue continue.
Note that if people were extending in nice ways, using defineProperty and making their props non-enumerable instead of just doing a [[Set]], that would significantly reduce issues like this...
Maybe. Though there would still be issues with implementations not aligning, like Ember which does use defineProperty to make them non-enumerable and doesn't pave existing methods, as well as issues with scripts that support pre-ES5 environments that don't want enumerable inconsistency.
JDD
On Tue, Sep 30, 2014 at 4:58 PM, Domenic Denicola < domenic at domenicdenicola.com> wrote:
This is a follow-up thread to discuss how to solve the problem identified in esdiscuss.org/topic/having-a-non-enumerable-array-prototype-contains-may-not-be-web-compatible without breaking the web. (Any arguments that it is OK to break sites using MooTools, please stay in that thread and respectfully do not reply to this one.)
So, guess that proves I should never say "this will be easy!" before working on a feature. Yay, the web.
I see a few options:
- Rename. The leading candidate would be
Array.prototype.has. I outlined in 1 whycontainsis better; note especially the DOM classes. But you could stretch things, especially by removing thefromIndexparameter, into an argument thathasis OK. (Basically, you'd be saying that an array is more like a set than a map, and that its analogy with String is mostly accidental.)
If we do rename, it should be almost anything other than .has. Arrays are clearly single-valued-mapping-like, not set-like, in that they map from indexes to values. If Array.prototype.has were to exist, it would need to test possible indexes.
- Specific hacks. I am thinking of e.g. making
Array.prototype.containsa getter, with a setter that does [[DefineOwnProperty]].
This could work, and it requires no new kernel mechanisms. If we do adopt this solution, the setter should be careful to play the same games that SES plays to work around the override mistake: If the this being set is not Array.prototype itself, the setter should use [[DefineOwnProperty]] to emulate an assignment to that this's own .contains.
- General hacks. I joked about @@unMooToolsables, but seriously, we could do something similar to @@unscopables of using MOP hooks to fix this problem. One idea that seems reasonable is @@enumerableWhenAssigned, so that
Array.prototype.containsstarts non-enumerable, but when you doArray.prototype.contains = x, it becomes enumerable. You could even generalize this into something that also fixes the override mistake 2, e.g. @@defineOwnPropertyOnAssign or @@assignIgnoresProto or similar. Or you could attack the problem at the for-in level
I suggest we focus on the override mistake. If we come up with a way of fixing it and .contains with one new kernel mechanism, that would be great. If we only fix the override mistake, still likely worth it. But if a new kernel mechanism only fixes .contains, it likely isn't worth it and we should return to #1 or #2.
1 is tempting in its simplicity. 2 is pretty gross and we'd probably be better of throwing out the feature. 3 is intriguing, although it obviously adds complexity cost to a feature that was supposed to be trivial.
I am curious if there is any support for 3, or proposals for a better hack in that vein. Because we will also have this problem for any other prototype extensions that work in the same way, e.g. MooTools's
flatten(which seems like something we'd likely want) or theirassociate,link,getRandom,combine, orpick(which seem less likely). And I assume the problem could extend beyond MooTools, and possibly beyond Array. (Although, Array is a special case in that we can't make any of its methods enumerable.)Especially if we choose something more general, e.g. @@assignIgnoresProto, I could see this being a powerful tool for fighting such problems in the future, and maybe for helping fix the override mistake (although I don't understand all of the use cases involved there).
The most painful use case is the existence of perfectly reasonable ES5 code like:
function Point(x, y) { this.x = x; this.y = y; }
Point.prototype.toString() { return `<${x},${y}>`; };
Because of the override mistake, this reasonable code no longer works after
Object.freeze(Object.prototype);
This sucks.
SES goes out of its way to not break code that follows ES5 best practices. The above Point code does. That's why SES's tamperProof(Object.prototype) replaces the data properties on Object.prototype with accessor properties whose setter uses [[DefineOwnProperty]] to emulate assignment on a this that is not Object.prototype itself.
With your #3, perhaps we'd have a less painful way to working around the override mistake.
Mark S. Miller wrote:
On Tue, Sep 30, 2014 at 4:58 PM, Domenic Denicola <domenic at domenicdenicola.com <mailto:domenic at domenicdenicola.com>> wrote:
I see a few options: 1. Rename. The leading candidate would be `Array.prototype.has`. I outlined in [1] why `contains` is better; note especially the DOM classes. But you could stretch things, especially by removing the `fromIndex` parameter, into an argument that `has` is OK. (Basically, you'd be saying that an array is more like a set than a map, and that its analogy with String is mostly accidental.)If we do rename, it should be almost anything other than .has. Arrays are clearly single-valued-mapping-like, not set-like, in that they map from indexes to values. If Array.prototype.has were to exist, it would need to test possible indexes.
Absolutely.
2. Specific hacks. I am thinking of e.g. making `Array.prototype.contains` a getter, with a setter that does [[DefineOwnProperty]].This could work, and it requires no new kernel mechanisms. If we do adopt this solution, the setter should be careful to play the same games that SES plays to work around the override mistake: If the this being set is not Array.prototype itself, the setter should use [[DefineOwnProperty]] to emulate an assignment to that this's own .contains.
Important safety tip with setters and Proxies -- Reflect helpers help.
3. General hacks. I joked about @@unMooToolsables, but seriously, we could do something similar to @@unscopables of using MOP hooks to fix this problem. One idea that seems reasonable is @@enumerableWhenAssigned, so that `Array.prototype.contains` starts non-enumerable, but when you do `Array.prototype.contains = x`, it becomes enumerable. You could even generalize this into something that also fixes the override mistake [2], e.g. @@defineOwnPropertyOnAssign or @@assignIgnoresProto or similar. Or you could attack the problem at the for-in levelI suggest we focus on the override mistake. If we come up with a way of fixing it and .contains with one new kernel mechanism, that would be great. If we only fix the override mistake, still likely worth it. But if a new kernel mechanism only fixes .contains, it likely isn't worth it and we should return to #1 or #2.
I wouldn't count on fixing the override mistake (inevitably by adding something new under the sun) to help extant code like MooTools, though.
The most painful use case is the existence of perfectly reasonable ES5 code like:
function Point(x, y) { this.x = x; this.y = y; } Point.prototype.toString() { return `<${x},${y}>`; };
You mean
Point.prototype.toString = function () { return ...; };
of course -- but you're using template string new syntax, so why not use Object.defineProperty here? Just sayin' ;-).
Because of the override mistake, this reasonable code no longer works after
Object.freeze(Object.prototype);This sucks.
SES goes out of its way to not break code that follows ES5 best practices. The above Point code does. That's why SES's tamperProof(Object.prototype) replaces the data properties on Object.prototype with accessor properties whose setter uses [[DefineOwnProperty]] to emulate assignment on a this that is not Object.prototype itself.
Yup, Domenic's #2.
With your #3, perhaps we'd have a less painful way to working around the override mistake.
I think #3, if hacked via @@enumerableWhenAssigned or any such thing, will just lead to more bugs. It's too implicit, modal.
Here's an alternative: add an assignment operator variant, spell it :=, that overrides. Boom, new code can work around the override mistake.
Point.prototype.toString := function () { return ...; };
Yeah, I remember := being mooted as [[DefineOwnProperty]] sugar taking a property descriptor, but I'm throwing this out here. It's simpler and does not confusingly vary the RHS to be a propdesc where regular assignment evaluates an arbitrary RHS assignment-expression.
Old code will need magic frozen-proto-setter hacks anyway. That ship sailed with ES5.
- I wasn't advocating to break the web but to not change the name because of a library bug, the initial thread spoiler was not needed
- the
@@enumerableWhenAssignedmight lead to shenanigans but it's polyfillable which is IMO preferable if it can solve somehow the problem
enumerableWhenAssigned(
Array.prototype,
'contains',
Array.prototype.contains
);
function enumerableWhenAssigned(obj, prop, value) {
Object.defineProperty(
obj,
prop,
{
configurable: true,
get: function () {
return value;
},
set: function (value) {
Object.defineProperty(
this,
prop,
{
value: value,
// writable: true, // maybe ?
configurable: true,
enumerable: true
}
);
}
}
);
}
my .02
Andrea Giammarchi wrote:
- I wasn't advocating to break the web but to not change the name because of a library bug, the initial thread spoiler was not needed
I wasn't responding to you, here on this thread (where you hadn't posted till now) or on the other one, so why are you jumping in and talking about what you were not advocating?
- the
@@enumerableWhenAssignedmight lead to shenanigans but it's polyfillable which is IMO preferable if it can solve somehow the problem
No need to write lots of code. The problem I see with these implicit/modal hacks is that they can easily break other code that doesn't want an override of a non-enumerable to be enumerable.
The magic proto-setter (Domenic's option 2) is "polyfillable", as Mark noted: no new kernel-language semantics required. It's also simpler than what you wrote, and doesn't risk blowback by breaking other code expecting non-enumerability to be preserved.
Le 1 oct. 2014 à 04:43, Mark S. Miller <erights at google.com> a écrit :
I suggest we focus on the override mistake. If we come up with a way of fixing it and .contains with one new kernel mechanism, that would be great. If we only fix the override mistake, still likely worth it. But if a new kernel mechanism only fixes .contains, it likely isn't worth it and we should return to #1 or #2.
I have thought about the possibility to have an "overridable" state attached to properties. That state may be implemented as an attribute, or as a mechanism à la @@unscopables.
(I have noted Brendan's reserve for such a mechanism; still, I think it is worth to consider how it could work with some details.)
The overridable state modifies the semantics of assignment (o.p = v) in the following ways (in short, it corrects override "mistakes"):
(1) it is not disallowed to set to a given property if there exists a nonwritable inherited property of same name. (2) overwriting a nonenumerable property will make it enumerable (unless the property is nonconfigurable, of course).
In case it is implemented à la @@unscopables, we could even, in order to minimise the API surface, decide that @@overridables === @@unscopables.
I expect that the overridable state won't need to be examined very often. Indeed, consider the following:
o.p = v
o.p = w
If the first assignement succeeds, the implementation doesn't need to check the overridable state in order to perform the second assignment, because the property will be (1) writable and (2) either enumerable or nonconfigurable.
Thanks Claude for filling in some of the details in my vague thinking. I guess I/we should try to come up with something more detailed, and see how/if it is able to address Mark's use cases, and how implementable it would be, and how extensively it would help (or not).
I tend to agree with Mark that unless this solves a larger problem, it does not pay its own way just for Array.prototype.contains (and potentially Array.prototype.flatten).
From: Claude Pache [mailto:claude.pache at gmail.com] Sent: Wednesday, October 1, 2014 09:36 To: Mark S. Miller Cc: Domenic Denicola; es-discuss at mozilla.org Subject: Re: Array.prototype.contains solutions
Le 1 oct. 2014 à 04:43, Mark S. Miller <erights at google.com<mailto:erights at google.com>> a écrit :
I suggest we focus on the override mistake. If we come up with a way of fixing it and .contains with one new kernel mechanism, that would be great. If we only fix the override mistake, still likely worth it. But if a new kernel mechanism only fixes .contains, it likely isn't worth it and we should return to #1 or #2.
I have thought about the possibility to have an "overridable" state attached to properties. That state may be implemented as an attribute, or as a mechanism à la @@unscopables.
(I have noted Brendan's reserve for such a mechanism; still, I think it is worth to consider how it could work with some details.)
The overridable state modifies the semantics of assignment (o.p = v) in the following ways (in short, it corrects override "mistakes"):
(1) it is not disallowed to set to a given property if there exists a nonwritable inherited property of same name. (2) overwriting a nonenumerable property will make it enumerable (unless the property is nonconfigurable, of course).
In case it is implemented à la @@unscopables, we could even, in order to minimise the API surface, decide that @@overridables === @@unscopables.
I expect that the overridable state won't need to be examined very often. Indeed, consider the following:
o.p = v
o.p = w
If the first assignement succeeds, the implementation doesn't need to check the overridable state in order to perform the second assignment, because the property will be (1) writable and (2) either enumerable or nonconfigurable.
The more I consider the issue, the more I think that the most elegant solution is to complete data properties with an additional "overridable" attribute, whose purpose is to refine the meaning of the "writable" attribute in the manner sketched below (in opposition of having an @@unscopables-like hack). At first glance, I expect that the addition of such an attribute should not trigger much BC issues, but that needs to be examined more closely.
—Claude
Le 1 oct. 2014 à 10:47, Domenic Denicola <domenic at domenicdenicola.com> a écrit :
Point 1 wasn't for your answer, I should have specified that, while the code was a playground for the magic setter in case MooTools guys would like to test a solution for contains or other methods too. Can't see how that could be smaller but it doesn't matter, it's small enough already.
On Wed, Oct 1, 2014 at 12:21 AM, Brendan Eich <brendan at mozilla.org> wrote:
The most painful use case is the existence of perfectly reasonable ES5
code like:
function Point(x, y) { this.x = x; this.y = y; } Point.prototype.toString() { return `<${x},${y}>`; };You mean
Point.prototype.toString = function () { return ...; };of course -- but you're using template string new syntax, so why not use Object.defineProperty here? Just sayin' ;-).
I did indeed mean the assignment in your correction.
Because of the override mistake, this reasonable code no longer works
after
Object.freeze(Object.prototype);This sucks.
SES goes out of its way to not break code that follows ES5 best practices. The above Point code does. That's why SES's tamperProof(Object.prototype) replaces the data properties on Object.prototype with accessor properties whose setter uses [[DefineOwnProperty]] to emulate assignment on a this that is not Object.prototype itself.
Yup, Domenic's #2.
With your #3, perhaps we'd have a less painful way to working around the
override mistake.
I think #3, if hacked via @@enumerableWhenAssigned or any such thing, will just lead to more bugs. It's too implicit, modal.
Here's an alternative: add an assignment operator variant, spell it :=, that overrides. Boom, new code can work around the override mistake.
Point.prototype.toString := function () { return ...; };Yeah, I remember := being mooted as [[DefineOwnProperty]] sugar taking a property descriptor, but I'm throwing this out here. It's simpler and does not confusingly vary the RHS to be a propdesc where regular assignment evaluates an arbitrary RHS assignment-expression.
Old code will need magic frozen-proto-setter hacks anyway. That ship sailed with ES5.
My concern is old code like Point co-existing with new framework code, like SES, that wants to freeze old prototypes, but is ignorant about all the particular old client abstractions, like Point, that need to work within that framework. The new framework code currently must use tamperProof rather than freeze, which is expensive and not a fully transparent fix. With #3, the new framework code might instead install an @@something on these frozen prototypes, leaving the properties on that prototype as data properties that do not suffer from the override mistake. The result could be efficient and adequately transparent.
Claude Pache wrote:
The more I consider the issue, the more I think that the most elegant solution is to complete data properties with an additional "overridable" attribute, whose purpose is to refine the meaning of the "writable" attribute in the manner sketched below (in opposition of having an @@unscopables-like hack).
A new attribute, per property, is safer in general (backward compatibility) than a per-object flag. A per-object list seems an obtuse way to encode an attribute, and I am quite sure implementations would prefer an attribute.
At first glance, I expect that the addition of such an attribute should not trigger much BC issues, but that needs to be examined more closely.
If it's opt-in, e.g. we define Array.prototype.contains with [[Overridable]] among its attributes, then at least MooTools works.
What could break? Code that did not expect 'contains' to exist on Array.prototype, but did expect assigning to someArray.contains would make a non-enumerable and/or non-own property. That would seem like nonsense or broken code, but it could exist for some name and prototypal relation, you're right.
Only way to find out is to implement and test at scale.
It would be good to hear from implementors on the idea of an [[Overridable]] attribute. SpiderMonkey has something similar but more restricted (see dxr.mozilla.org/mozilla-central/source/js/src/vm/Shape.h?from=SHADOWABLE&case=true#1013).
Does the [[Overridable]] you have in mind also fix the override mistake? It sounds like it should.
Regarding shadowable, is there any better documentation than the doc-comment:
/* * For ES5 compatibility, we allow properties with PropertyOp-flavored * setters to be shadowed when set. The "own" property thereby created in * the directly referenced object will have the same getter and setter as * the prototype property. See bug 552432. */
Is there any observable effect of shadowable? Example?
Mark S. Miller wrote:
Does the [[Overridable]] you have in mind also fix the override mistake? It sounds like it should.
Yes, definitely.
Regarding shadowable, is there any better documentation than the doc-comment:
|/*| | * For ES5 compatibility, we allow properties with PropertyOp-flavored| | * setters to be shadowed when set. The "own" property thereby created in| | * the directly referenced object will have the same getter and setter as| | * the prototype property. Seebug 552432.| | */|
Is there any observable effect of shadowable? Example?
The comment is a bit misleading. The compatibility at stake is old SpiderMonkey C API, by which the DOM can make pseudo-data properties that have native (C function) get and set accessors, but which should appear to be overridable (as data properties are, if writable). I'm not sure if I wrote "ES5" there or someone else did, but ES5 helped straighten out old internal/pre-standard SpiderMonkey interfaces, and collided under the hood with the pseudo-data property support.
I'd love to be able to get rid of SHADOWABLE, actually! I just recently found out we can do it with a little rearranging. Extra state-space in property attributes is unavoidable complexity for us. My own preference would be not to standardize it. :)
There's also the [Replaceable] attribute in WebIDL, which arises from similar compatibility needs. It's implemented as a setter that redefines the property being set.
This is a bad bunch of options. I think renaming ("hasValue"?) is probably least-worst, followed by waiting a year and trying again.
Jason Orendorff wrote:
I'd love to be able to get rid of SHADOWABLE, actually! I just recently found out we can do it with a little rearranging. Extra state-space in property attributes is unavoidable complexity for us. My own preference would be not to standardize it.:)
There's also the [Replaceable] attribute in WebIDL, which arises from similar compatibility needs. It's implemented as a setter that redefines the property being set.
"Nature" is trying to tell you something :-P.
This is a bad bunch of options. I think renaming ("hasValue"?) is probably least-worst, followed by waiting a year and trying again.
Maybe, but if we fail then, we can't keep waiting. ES7 is 2015-2016, no slipping that train.
I faintly remember seeing a sort of "proposal" containing many different approaches as how to solve this generic built-ins extensions problem in future editions of ECMAScript. I believe it was authored by Domenic Denicola, in Gist/GitHub format if memory serves me right.
I can't seem to find it, does anyone have a link? Or is memory tricking me and this thread was the end point for this topic? Thanks.
www.google.com/search?q=site%3Aesdiscuss.org+Array.prototype.contains+solutions leads to esdiscuss.org/topic/array-prototype-contains-solutions which is probably what you were thinking of.
They were not very good ideas.
Domenic Denicola wrote:
www.google.com/search?q=site%3Aesdiscuss.org+Array.prototype.contains+solutions leads tohttps://esdiscuss.org/topic/array-prototype-contains-solutions which is probably what you were thinking of.
They were not very good ideas.
It's a hard problem. Original-JS2/ES4 had namespaces, after Common Lisp symbol packages, as a proposed solution. You'd have to open a namespace, e.g.
use namespace intrinsic; if (myArray.contains(someValue)) ...;
or explicitly quality:
if (myArray.intrinsic::contains(someValue)) ...;
in case you needed to mix MooTools' contains with the new built-in one.
Namespaces were decisively rejected from the Harmony agenda at its founding in Oslo, July 2008:
General hacks tend to generalize to namespaces if you push them. Specific hacks as you say are not very good ideas. This leaves picking good-enough names that don't collide. AFAIK this is the state of the art. Just recapitulating for anyone new to es-discuss, and calling for better ideas. Thanks,
www.google.com/search?q=site%3Aesdiscuss.org+Array.prototype.contains+solutions leads to esdiscuss.org/topic/array-prototype-contains-solutions which is probably what you were thinking of.
They were not very good ideas.
For some reason I thought you had evolved those ideas further after that post, but if you don't recall it then my memory was wrong. Thanks for the quick answer.
This is a follow-up thread to discuss how to solve the problem identified in esdiscuss.org/topic/having-a-non-enumerable-array-prototype-contains-may-not-be-web-compatible without breaking the web. (Any arguments that it is OK to break sites using MooTools, please stay in that thread and respectfully do not reply to this one.)
So, guess that proves I should never say "this will be easy!" before working on a feature. Yay, the web.
I see a few options:
Rename. The leading candidate would be
Array.prototype.has. I outlined in 1 whycontainsis better; note especially the DOM classes. But you could stretch things, especially by removing thefromIndexparameter, into an argument thathasis OK. (Basically, you'd be saying that an array is more like a set than a map, and that its analogy with String is mostly accidental.)Specific hacks. I am thinking of e.g. making
Array.prototype.containsa getter, with a setter that does [[DefineOwnProperty]].General hacks. I joked about @@unMooToolsables, but seriously, we could do something similar to @@unscopables of using MOP hooks to fix this problem. One idea that seems reasonable is @@enumerableWhenAssigned, so that
Array.prototype.containsstarts non-enumerable, but when you doArray.prototype.contains = x, it becomes enumerable. You could even generalize this into something that also fixes the override mistake 2, e.g. @@defineOwnPropertyOnAssign or @@assignIgnoresProto or similar. Or you could attack the problem at the for-in level1 is tempting in its simplicity. 2 is pretty gross and we'd probably be better of throwing out the feature. 3 is intriguing, although it obviously adds complexity cost to a feature that was supposed to be trivial.
I am curious if there is any support for 3, or proposals for a better hack in that vein. Because we will also have this problem for any other prototype extensions that work in the same way, e.g. MooTools's
flatten(which seems like something we'd likely want) or theirassociate,link,getRandom,combine, orpick(which seem less likely). And I assume the problem could extend beyond MooTools, and possibly beyond Array. (Although, Array is a special case in that we can't make any of its methods enumerable.)Especially if we choose something more general, e.g. @@assignIgnoresProto, I could see this being a powerful tool for fighting such problems in the future, and maybe for helping fix the override mistake (although I don't understand all of the use cases involved there).