adopted-ember-addons / ember-data-model-fragments

Ember Data addon to support nested JSON documents
MIT License
370 stars 114 forks source link

Preserve fragment array state after the record is unloaded #488

Closed dwickern closed 3 months ago

dwickern commented 3 months ago

When unloading a record, ember-data's @attr, @belongsTo and @hasMany preserve the record's most recent state, even though the underlying RecordData has been reset.

This project's @fragment and @array also work this way, but there's a problem with @fragmentArray.

Reproduction

Consider this component using the order and product models from this project's test suite:

import Component from '@glimmer/component';
import { service } from '@ember/service';
import { action } from '@ember/object';

export default class OrderComponent extends Component {
  @service router;

  @action async delete(order) {
    await order.destroyRecord();
    this.router.transitionTo('/orders');
  }

  <template>
    <ul>
      {{#each @order.products as |product|}}
        <li>{{product.name}}: {{product.price}}</li>
      {{/each}}
    </ul>

    <button type="button" {{on "click" (fn this.delete @order)}}>
      Delete
    </button>
  </template>
}

The products will initially render

<ul>
    <li>The Strangler: 299.99</li>
    <li>Tears of Lys: 499.99</li>
</ul>

Clicking the delete button calls destroyRecord which does a delete + save, then unloads the record and all its fragments. Both products are unloaded.

But we haven't transitioned away yet; we're still rendering order.products. When the UI re-renders, the products no longer exist so the FragmentArray will create two new products, each in an empty state (all properties undefined). For a brief moment before transitioning away, the products will render as

<ul>
    <li>:</li>
    <li>:</li>
</ul>

Problem

In that scenario, we've done unnecessary re-rendering, displayed visual artifacts to the user, and created some unused fragment records (a possible memory leak). In practice it causes all kinds of errors in my app, because model classes have other logic which doesn't anticipate the model being created with no properties.

The problem is that FragmentArray stores a currentState: RecordData[] and expects ember-data to produce the Fragment record on demand from its cache each time objectAt is called: https://github.com/adopted-ember-addons/ember-data-model-fragments/blob/316e9be882baf3a9b20b8d6243de3e9fb518beb0/addon/array/fragment.js#L31-L37 https://github.com/adopted-ember-addons/ember-data-model-fragments/blob/316e9be882baf3a9b20b8d6243de3e9fb518beb0/addon/record-data.js#L1001-L1009

When the records are unloaded, they're removed from ember-data's cache, then FragmentArray recreates them the next time they're accessed.

Solution

My solution is for FragmentArray to store currentState: Fragment[] instead. When the FragmentArray is destroyed, it will stop retrieving state from its RecordData, effectively becoming a read-only snapshot of the latest state.

knownasilya commented 3 months ago

Released as v6.0.10