Ember.js: [3.6.0] Regression when creating a MutableArray implementation

[boris-petrov]

We have the following code that works fine in 3.5.1:

  const properties = {
    ...
    replace(index, amount, objects = []) {
      // do some special magic
      return this;
    },
  };

  const result = Mixin
    .create(MutableArray, Observable, properties)
    .without(['length'])
    .apply([]);

And this worked fine - e.g. when calling result.pushObject or result.insertAt or result.removeAt that would call our version of replace and we could handle it specially. This is broken in 3.6.0. I see here that a "generic" replace function is called while in 3.5.1 it was implemented in a different way and that's why what we did worked before. Not sure if we were using private APIs or something which was never supported but I think I've read in the documentation that when you want to create your own type of array, you should just implement replace and all else will work with it. That seemed to be true before but now it is broken. Is that OK? How are we supposed to do something like what we were doing before?

[pixelhandler]

@boris-petrov I see that replace is documented here:

• https://emberjs.com/api/ember/3.5/classes/MutableArray/methods/replace?anchor=replace
• https://github.com/emberjs/ember.js/blob/v3.6.0/packages/%40ember/-internals/runtime/lib/mixins/array.js#L1607-L1614

Which appears to be the same as an 3.6

• https://github.com/emberjs/ember.js/blob/v3.6.0/packages/%40ember/-internals/runtime/lib/mixins/array.js#L1191-L1208

I don't know how this is used...

• https://github.com/emberjs/ember.js/blob/v3.6.0/packages/%40ember/-internals/runtime/lib/mixins/array.js#L1607-L1614

What I think will be helpful is if you can create simplified reproduction of the issue, perhaps in an ember-twiddle or a public repo.

[boris-petrov]

Well, here is a reproduction. Run ember serverand open the page. You won't see the console.log in the console. If you checkout the previous commit (before the upgrade to Ember 3.6) and run the server, you'll see it print in the console.

This is a showstopper for us. Is it a bug or is this expected and we were using something private that changed?

[rwjblue]

I’ll try to poke at the repro later today (assuming I don’t freeze, fall off a ladder, or electrocute myself while hanging holiday lights 😝)....

[devdemi]

I get next error message in console:

rsvp.js:24 Uncaught RangeError: Maximum call stack size exceeded at WeakMap.get () at getCacheFor (metal.js:25) at ComputedProperty.get (metal.js:2350) at Array.CPGETTER_FUNCTION [as []] (metal.js:752) at Function.jQuery.extend.jQuery.fn.extend (jquery.js:261) at Function.jQuery.extend.jQuery.fn.extend (jquery.js:282) at Function.jQuery.extend.jQuery.fn.extend (jquery.js:282) at Function.jQuery.extend.jQuery.fn.extend (jquery.js:282) at Function.jQuery.extend.jQuery.fn.extend (jquery.js:282) at Function.jQuery.extend.jQuery.fn.extend (jquery.js:282)

I figured out that reason of it store.query request in init in one of my components:

this.get('store').query('user', { 'filter': { 'type': '1', 'gender': ['male', 'female'] } }).then(tutors => {
    this.set('tutors', tutors);
});

Error occurs when I try to pass an array to filter.
With Ember 3.5.1 it works well.

[boris-petrov]

@rwjblue - did you get a chance to see the reproduction and do you have any ideas what we can do? We cannot update to 3.6 because of that. :(

[jacobq]

@devdemi what you mentioned sounds related to https://github.com/emberjs/ember.js/issues/17373 which should be fixed in 3.6.1, which was just released. (I haven't tested DS.RecordArray/ArrayProxy but NativeArray, which is used to extend Array.prototype no longer has the self-reference / infinite loop problem.)

[boris-petrov]

@jacobq - any idea about the original issue? It's not fixed in 3.6.1 unfortunately.

[jacobq]

@boris-petrov Unfortunately, no, most of the time I can only really justify working on issues that affect my clients, and I haven't looked into this problem. However, you seem to have written a fairly insightful explanation and provided a repro, so I suspect that one of the ember.js team members will take a stab at it soon. The only thing I can think to add would be to use git bisect to track down the first commit since 3.5.1 where this problem started happening. That seemed to help speed up resolution of #17190. (You could do almost the same thing -- just changing the node -e... line to do your test instead. If it gets cumbersome to do inline like that you could put it in a .js file instead.)

[boris-petrov]

Thanks for the response. It's probably easy to find which commit did it - right now the code is written to not support our use case so I just have to find out what did it. I'll write tomorrow so if anyone wants to help, they can start from there. :)

[boris-petrov]

I think this is the commit that breaks it. Not sure what the rationale is for it. @bekzod - could you step in please? Why was this done and can you revert it/modify it so our use-case is fixed? I think this is a breaking change with no apparent workaround (apart from overriding all of those functions myself and handling them as they were handled in 3.5.*).

P.S. A real easy fix would to just remove the replace import and change the two replace calls to array.replace. Not sure if that is correct but sounds reasonable to me.