Preserve fragment array state after the record is unloaded (#488)

* add tests for record state after unload

* preserve fragment array state after unload

* only notify on changed keys

* add assertion when updating a destroyed fragment array

addon/array/fragment.js

@@ -28,14 +28,6 @@ const FragmentArray = StatefulArray.extend({
   */
   modelName: null,
 
-  objectAt(index) {
-    const recordData = this._super(index);
-    if (recordData === undefined) {
-      return;
-    }
-    return recordData._fragmentGetRecord();
-  },
-
   _normalizeData(data, index) {
     assert(
       `You can only add '${this.modelName}' fragments or object literals to this property`,
@@ -46,14 +38,28 @@ const FragmentArray = StatefulArray.extend({
     if (isFragment(data)) {
       const recordData = recordDataFor(data);
       setFragmentOwner(data, this.recordData, this.key);
-      return recordData;
+      return recordData._fragmentGetRecord();
     }
-    const existing = this.objectAt(index);
+    const existing = this.currentState[index];
     if (existing) {
       existing.setProperties(data);
-      return recordDataFor(existing);
+      return existing;
     }
-    return this.recordData._newFragmentRecordDataForKey(this.key, data);
+    const recordData = this.recordData._newFragmentRecordDataForKey(
+      this.key,
+      data
+    );
+    return recordData._fragmentGetRecord();
+  },
+
+  _getFragmentState() {
+    const recordDatas = this._super();
+    return recordDatas?.map((recordData) => recordData._fragmentGetRecord());
+  },
+
+  _setFragmentState(fragments) {
+    const recordDatas = fragments.map((fragment) => recordDataFor(fragment));
+    this._super(recordDatas);
   },
 
   /**

addon/array/stateful.js

@@ -106,11 +106,23 @@ const StatefulArray = EmberObject.extend(MutableArray, Copyable, {
     return data;
   },
 
+  _getFragmentState() {
+    return this.recordData.getFragment(this.key);
+  },
+
+  _setFragmentState(array) {
+    this.recordData.setDirtyFragment(this.key, array);
+  },
+
   replace(start, deleteCount, items) {
     assert(
       'The third argument to replace needs to be an array.',
       isArray(items)
     );
+    assert(
+      'Attempted to update the fragment array after it was destroyed',
+      !this.isDestroyed && !this.isDestroying
+    );
     if (deleteCount === 0 && items.length === 0) {
       // array is unchanged
       return;
@@ -124,7 +136,7 @@ const StatefulArray = EmberObject.extend(MutableArray, Copyable, {
       deleteCount,
       ...items.map((item, i) => this._normalizeData(item, start + i))
     );
-    this.recordData.setDirtyFragment(this.key, data);
+    this._setFragmentState(data);
     this.notify();
   },
 
@@ -133,9 +145,9 @@ const StatefulArray = EmberObject.extend(MutableArray, Copyable, {
     if (this.isDestroyed || this.isDestroying || this._isUpdating) {
       return;
     }
-    const currentState = this.recordData.getFragment(this.key);
+    const currentState = this._getFragmentState();
     if (currentState == null) {
-      // detached
+      // detached; the underlying fragment array was set to null after this StatefulArray was accessed
       return;
     }
 

addon/record-data.js

@@ -752,9 +752,9 @@ export default class FragmentRecordData extends RecordData {
 
       Object.assign(this._fragmentData, newCanonicalFragments);
       // update fragment arrays
-      changedFragmentKeys?.forEach((key) =>
-        this._fragmentArrayCache[key]?.notify()
-      );
+      changedFragmentKeys?.forEach((key) => {
+        this._fragmentArrayCache[key]?.notify();
+      });
     }
 
     const changedKeys = mergeArrays(changedAttributeKeys, changedFragmentKeys);
@@ -830,9 +830,9 @@ export default class FragmentRecordData extends RecordData {
     const changedAttributeKeys = super.didCommit(data);
 
     // update fragment arrays
-    Object.keys(newCanonicalFragments).forEach((key) =>
-      this._fragmentArrayCache[key]?.notify()
-    );
+    changedFragmentKeys.forEach((key) => {
+      this._fragmentArrayCache[key]?.notify();
+    });
 
     const changedKeys = mergeArrays(changedAttributeKeys, changedFragmentKeys);
     if (gte('ember-data', '4.5.0') && changedKeys?.length > 0) {

tests/integration/render_test.js

@@ -4,18 +4,21 @@ import { render, settled } from '@ember/test-helpers';
 import { hbs } from 'ember-cli-htmlbars';
 import { setComponentTemplate } from '@ember/component';
 import Component from '@glimmer/component';
+import Pretender from 'pretender';
 
 module('Integration | Rendering', function (hooks) {
   setupRenderingTest(hooks);
 
-  let store;
+  let store, server;
 
   hooks.beforeEach(function () {
     store = this.owner.lookup('service:store');
+    server = new Pretender();
   });
 
   hooks.afterEach(function () {
     store = null;
+    server.shutdown();
   });
 
   test('construct fragments without autotracking.mutation-after-consumption error', async function (assert) {
@@ -194,4 +197,118 @@ module('Integration | Rendering', function (hooks) {
     assert.dom('[data-product]').exists({ count: 1 });
     assert.dom('[data-product="0"]').hasText('The Strangler: 299.99');
   });
+
+  test('fragment is destroyed', async function (assert) {
+    this.order = store.createRecord('order', { id: 1 });
+
+    store.push({
+      data: [
+        {
+          id: this.order.id,
+          type: 'order',
+          attributes: {
+            product: {
+              name: 'The Strangler',
+              price: '299.99',
+            },
+          },
+          relationships: {},
+        },
+      ],
+    });
+
+    await render(hbs`
+      {{#let this.order.product as |product|}}
+        <span data-product>{{product.name}}: {{product.price}}</span>
+      {{/let}}
+    `);
+
+    assert.dom('[data-product]').hasText('The Strangler: 299.99');
+
+    server.delete('/orders/1', () => [204]);
+    await this.order.destroyRecord();
+
+    assert.dom('[data-product]').hasText('The Strangler: 299.99');
+  });
+
+  test('fragment array is destroyed', async function (assert) {
+    this.order = store.createRecord('order', { id: 1 });
+
+    store.push({
+      data: [
+        {
+          id: this.order.id,
+          type: 'order',
+          attributes: {
+            products: [
+              {
+                name: 'The Strangler',
+                price: '299.99',
+              },
+              {
+                name: 'Tears of Lys',
+                price: '499.99',
+              },
+            ],
+          },
+          relationships: {},
+        },
+      ],
+    });
+
+    await render(hbs`
+      <ul>
+        {{#each this.order.products as |product idx|}}
+          <li data-product="{{idx}}">{{product.name}}: {{product.price}}</li>
+        {{/each}}
+      </ul>
+    `);
+
+    assert.dom('[data-product]').exists({ count: 2 });
+    assert.dom('[data-product="0"]').hasText('The Strangler: 299.99');
+    assert.dom('[data-product="1"]').hasText('Tears of Lys: 499.99');
+
+    server.delete('/orders/1', () => [204]);
+    await this.order.destroyRecord();
+
+    assert.dom('[data-product]').exists({ count: 2 });
+    assert.dom('[data-product="0"]').hasText('The Strangler: 299.99');
+    assert.dom('[data-product="1"]').hasText('Tears of Lys: 499.99');
+  });
+
+  test('array destroyed', async function (assert) {
+    this.person = store.createRecord('person', { id: 1 });
+
+    store.push({
+      data: [
+        {
+          id: this.person.id,
+          type: 'person',
+          attributes: {
+            titles: ['Hand of the King', 'Master of Coin'],
+          },
+          relationships: {},
+        },
+      ],
+    });
+
+    await render(hbs`
+      <ul>
+        {{#each this.person.titles as |title idx|}}
+          <li data-title="{{idx}}">{{title}}</li>
+        {{/each}}
+      </ul>
+    `);
+
+    assert.dom('[data-title]').exists({ count: 2 });
+    assert.dom('[data-title="0"]').hasText('Hand of the King');
+    assert.dom('[data-title="1"]').hasText('Master of Coin');
+
+    server.delete('/people/1', () => [204]);
+    await this.person.destroyRecord();
+
+    assert.dom('[data-title]').exists({ count: 2 });
+    assert.dom('[data-title="0"]').hasText('Hand of the King');
+    assert.dom('[data-title="1"]').hasText('Master of Coin');
+  });
 });

tests/unit/array_property_test.js

@@ -230,6 +230,42 @@ module('unit - `MF.array` property', function (hooks) {
     );
   });
 
+  test('preserve fragment array when record is unloaded', async function (assert) {
+    store.push({
+      data: {
+        type: 'person',
+        id: 1,
+        attributes: {
+          titles: ['Hand of the King', 'Master of Coin'],
+        },
+      },
+    });
+
+    const person = await store.findRecord('person', 1);
+    const titles = person.titles;
+
+    assert.strictEqual(titles.length, 2);
+
+    const titleBefore = titles.objectAt(0);
+    assert.strictEqual(titleBefore, 'Hand of the King');
+
+    person.unloadRecord();
+
+    assert.strictEqual(
+      person.titles,
+      titles,
+      'StatefulArray instance is the same after unload'
+    );
+
+    const titleAfter = titles.objectAt(0);
+
+    assert.strictEqual(
+      titleAfter,
+      titleBefore,
+      'preserve array contents after unload'
+    );
+  });
+
   if (HAS_ARRAY_OBSERVERS) {
     test('supports array observers', async function (assert) {
       store.push({

tests/unit/fragment_array_property_test.js

@@ -586,6 +586,38 @@ module('unit - `MF.fragmentArray` property', function (hooks) {
     });
   });
 
+  test('preserve fragment array when record is unloaded', async function (assert) {
+    pushPerson(1);
+
+    const person = await store.findRecord('person', 1);
+    const addresses = person.addresses;
+    assert.strictEqual(addresses.length, 2);
+
+    const addressBefore = addresses.objectAt(0);
+    assert.strictEqual(addressBefore.street, '1 Sky Cell');
+
+    person.unloadRecord();
+
+    assert.strictEqual(
+      person.addresses,
+      addresses,
+      'fragment array instance is the same after unload'
+    );
+
+    const addressAfter = addresses.objectAt(0);
+
+    assert.strictEqual(
+      addressAfter,
+      addressBefore,
+      'FragmentArray instance is the same after unload'
+    );
+    assert.strictEqual(
+      addressAfter.street,
+      '1 Sky Cell',
+      'preserve fragment attributes after unload'
+    );
+  });
+
   test('pass arbitrary props to createFragment', async function (assert) {
     pushPerson(1);
 

tests/unit/fragment_property_test.js

@@ -521,6 +521,37 @@ module('unit - `MF.fragment` property', function (hooks) {
     });
   });
 
+  test('preserve fragment attributes when record is unloaded', async function (assert) {
+    store.push({
+      data: {
+        type: 'person',
+        id: 1,
+        attributes: {
+          name: {
+            first: 'Barristan',
+            last: 'Selmy',
+          },
+        },
+      },
+    });
+
+    const person = await store.findRecord('person', 1);
+    const name = person.name;
+
+    person.unloadRecord();
+
+    assert.strictEqual(
+      person.name,
+      name,
+      'Fragment instance is the same after unload'
+    );
+    assert.strictEqual(
+      name.first,
+      'Barristan',
+      'preserve fragment attributes after unload'
+    );
+  });
+
   test('pass arbitrary props to createFragment', async function (assert) {
     const address = store.createFragment('address', {
       street: '1 Dungeon Cell',