[Possible 3.15 regression] Model Fragment's fragmentArray throws backtracking rerender assertion from 3.15.x

[patocallaghan]

Our app relies heavily on the addon ember-data-model-fragments, an addon which integrates with Ember Data to allow you to treat blobs of JSON as models. I'm currently trying to upgrade our app from ember-source 3.14.x to 3.15.x but I've found a blocker which is preventing our upgrade due to a change in ember-source 3.15. (I've also tested with v3.17.0-beta.6 and it's still the same)

This issue is a similar shape to the issue "Possible 3.15/Octane regression with ember-table addon", backtracking rerender assertions have started triggered due to the ember-source changes. I guess what I’m trying to figure out:

1. Is this is a valid triggering of the backtracking rerender assertion? Or ...
2. Is this an edge-case bug and the assertion shouldn't be triggered?

If this is a case where ember-data-model-fragments is doing bad stuff, I’d like to try and understand how to fix to unblock myself 😃

From ember-source 3.15.0-beta.4 it appears that tracked models which have properties using fragmentArray will throw a backtracking rerender assertion causing things to blow up.

There are two suspect commits in ember-source where this was introduced.

#17834 [BUGFIX] Prevents autotracking ArrayProxy creation
#18554 [BREAKING BUGFIX] Adds autotracking transaction

I think it's #17834 which introduced it (cc-ing @pzuraq who made this change).

I've created an extremely simplified test case in adopted-ember-addons/ember-data-model-fragments#358 and it throws the following assertion:

Error: Assertion Failed: You attempted to update `[]` on `Array`, but it had already been used previously in the same computation.  Attempting to update a value after using it in a computation can cause logical errors, infinite revalidation bugs, and performance issues, and is not supported.

`[]` was first used:

- While rendering:
  
  application
    index
      my-component
        this.record

Stack trace for the first usage: 
    at get (http://localhost:7357/assets/vendor.js:14851:32)
    at Class._addArrangedContentArrayObserver (http://localhost:7357/assets/vendor.js:28971:44)
    at Class.init (http://localhost:7357/assets/vendor.js:28831:12)
    at Class.init (http://localhost:7357/assets/vendor.js:87960:12)
    at Class.superWrapper [as init] (http://localhost:7357/assets/vendor.js:30845:22)
    at initialize (http://localhost:7357/assets/vendor.js:29137:9)
    at Function.create (http://localhost:7357/assets/vendor.js:29720:9)
    at createFragmentArray (http://localhost:7357/assets/vendor.js:88297:32)

Stack trace for the update:
    at dirtyTagFor (http://localhost:7357/assets/vendor.js:55641:11)
    at markObjectAsDirty (http://localhost:7357/assets/vendor.js:14006:32)
    at notifyPropertyChange (http://localhost:7357/assets/vendor.js:14043:5)
    at arrayContentDidChange (http://localhost:7357/assets/vendor.js:14144:7)
    at replaceInNativeArray (http://localhost:7357/assets/vendor.js:14210:5)
    at Array.replace (http://localhost:7357/assets/vendor.js:27255:39)
    at Class.setupData (http://localhost:7357/assets/vendor.js:88000:15)

The problem appears to happen in the following lines in ember-data-model-fragments, specifically 298 (where the array is created) and 300 (where the array is updated) https://github.com/lytics/ember-data-model-fragments/blob/c00e2b1a40b296e0c0c3c21ca0f463d12888cd27/addon/attributes.js#L298-L300

The array is first set up in createArray at

https://github.com/lytics/ember-data-model-fragments/blob/c00e2b1a40b296e0c0c3c21ca0f463d12888cd27/addon/attributes.js#L195-L208

the array is modified a second time in

https://github.com/lytics/ember-data-model-fragments/blob/f1c7060fb7560aaea26760e0270234823b9e20db/addon/array/stateful.js#L79

In my test reproduction I am specifically marking the property as @tracked

@tracked selectedRecord = this.store.createRecord('my-model', { fragments: [] });

[pzuraq]

So, this isn't intended, and we dug in and added it to the fix for #18770. Once that lands, it should prevent creating an ArrayProxy from entangling and causing this error.

However, I ran that branch against your test and it does still fail. This is because Model Fragments is also entangling the content array multiple places before mutating it, within its own code. Whenever you see get(this, 'content'), that will entangle the array, and then when you try to update it later it will throw a new error.

So, Model Fragments will likely still need to be refactored after we merge the fix for the Ember side of things.

In the long term, I think in general Model Fragments and other users should start moving away from ArrayProxy in general. Instead, they should be moving toward creating their own custom iterables. Native iterables should be supported within templates, though that does require Symbol support for now. They should also autotrack properly in general, as long as they're properly instrumented.

If native iterables aren't an option due to IE11 support, Ember also does support iterating over any object that has a forEach method. This method is a bit less well known, we should work on documenting it more, but I think this would be a good way to move toward that.

Happy to talk this through at some point in more detail, just let me know!

[patocallaghan]

@pzuraq Thanks for the detailed breakdown 👍Just one question, what exactly does the term "entangling" refer to in this context?

Also yep, it'd be great to chat about the other problems in Model Fragments & ArrayProxy. I'll follow up with you on Discord early next week.

[rwjblue]

Closed by #18770