archives

« Bugzilla Issues Index

#3256 — New Array.prototype behavior of returning an instance of `this.constructor` breaks Zepto.


We have discovered a breaking change in ES6 that has significant impact on the web (especially the mobile web). Zepto is affected and possibly other libraries that depend on similar patterns.

The problem arises because Array.prototype methods now attempt to construct an instance of this.constructor rather than Array. This is obviously beneficial for some cases, but has extremely bad behavior for libraries like Zepto that detach methods from Array.prototype and apply them to objects whose only constructor property is found on Object.prototype.

The min repro case is something like this:

var obj = [1,2,3];
obj.__proto__ = { slice: Array.prototype.slice };
var res = obj.slice(2);
Array.isArray(res); // true in ES5, false in ES6.

Key pieces of code:

Zepto prototype object: https://github.com/madrobby/zepto/blob/master/src/zepto.js#L389-L796
(note lack of constructor property)

Zepto "wrapper": https://github.com/madrobby/zepto/blob/master/src/zepto.js#L157-L162

Example isArray check that often fails in practice with ES6 semantics: https://github.com/madrobby/zepto/blob/master/src/zepto.js#L198

The scope of this breakage seems very large. If at all possible we should update the semantics to preserve backwards compat for Zepto.


If we only have to worry about implicit inheritance of "constructor" from Object.prototype we can probably fix it by inserting an additional step between 14.c.ii (which is itself an hack to fix an earlier compat issue) and 14.c.ii.1. The new line is an additional guard on using C as the constructor:
If SameValue(C, %Object%) is false, then
(existing line 14.c.ii.1)

This takes care of the sloppy code like what you presented that "accidentally" returns %Object% as the value of obj.constructor. It would still break anybody who setups a well formed "class" using array methods and then did the same sort of dunder proto hacking to turn exotic array instances into instances of that class.

I think I have a fix for that case too, but it would require exposing an additional @@method. I not yet sure we really need it.

Here is a sketch of the fix:
Define a new "static" accessor property on Array whose getter is essentintially:
function () {return this}.

All occurenencnce in Array.prototype methods of sequences that look like:
4. If O is an exotic Array object, then
a. Let C be Get(O, "constructor").
b. ReturnIfAbrupt(C).
c. If IsConstructor(C) is true, then
i. Let thisRealm be the running execution context’s Realm.
ii. If SameValue(thisRealm, GetFunctionRealm(C)) is true, then
1. Let A be the result of calling the [[Construct]] internal method of C with argument (0).
5. If A is undefined, then
a. Let A be ArrayCreate(0).

would be replaced by:
4. Let C be Get(O, "constructor").
5. ReturnIfAbrupt(C).
6. If IsConstructor(C) is true, then
a. Let species be Get(O, @@species);
b. If IsConstructor(species) is true, then
i. Let C be species.
c. Else, let C be undefined.
7. If IsConstructor(C) is true, then
a. Let thisRealm be the running execution context’s Realm.
b. If SameValue(thisRealm, GetFunctionRealm(C)) is true, then
i. Let A be the result of calling the [[Construct]] internal method of C with argument (0).
8. If A is undefined, then
a. Let A be ArrayCreate(0).

This approach actually is more backwards compatible and also provides greater flexibility for ES6 level programmers who what to wire together new kinds of pseudo-array classes and reuse Array prototype methods.


(In reply to Allen Wirfs-Brock from comment #1)

Bug fixed in line 6a below

>
>
> would be replaced by:
> 4. Let C be Get(O, "constructor").
> 5. ReturnIfAbrupt(C).
> 6. If IsConstructor(C) is true, then
> a. Let species be Get(C, @@species);
> b. If IsConstructor(species) is true, then
> i. Let C be species.
> c. Else, let C be undefined.
> 7. If IsConstructor(C) is true, then
> a. Let thisRealm be the running execution context’s Realm.
> b. If SameValue(thisRealm, GetFunctionRealm(C)) is true, then
> i. Let A be the result of calling the [[Construct]] internal method
> of C with argument (0).
> 8. If A is undefined, then
> a. Let A be ArrayCreate(0).
>


Here's a slightly refactored version that should be semantically identical (except for the added ReturnIfAbrupt check after getting @@species from C).

4. Let C be Get(O, "constructor").
5. ReturnIfAbrupt(C).
6. If IsConstructor(C) is true, then
a. Let species be Get(C, @@species);
b. ReturnIfAbrupt(species)
c. If IsConstructor(species) is true, then
i. let thisRealm be the running execution context’s Realm.
ii. If SameValue(thisRealm, GetFunctionRealm(C)) is true, then
1. Let A be the result of calling the [[Construct]] internal method of species with argument (0).
7. If A is undefined, then
a. Let A be ArrayCreate(0).

Allen currently investigating whether the realm check needs to be moved to 6.a before checking species.


fixed in rev29 editor's draft


fixed in rev29