Add support for nested saves

[hjdivad]

Rendered

[sandstrom]

@hjdivad @igorT I'm thrilled to see this RFC! 🏅

Of all open issues in Ember Data this is the one with the most comments (not counting duplicates). So it's definitely something that would bring joy to a huge amount of Ember Data users!

Some thoughts:

• How about using a key other than clientId, something like tempId or ephemeralId to signify that it's a temporary value? Nothing major though, clientId totally works — this comment is in the bike-shed category.
• Is this supposed to work together with EmbeddedRecordsMixin or as an alternative? If it's supposed to work along-side, it would be neat if the logic provided by savedWith could be hidden. I.e. embedded records as defined by the EmbeddedRecordsMixin would always get this behaviour. Or am I missing some obvious drawback with that?
• Good that you bring up out-of-band websockets, this has been an issue in our app (we've worked around it with server-logic, but this would be much cleaner).
• With out-of-band, presumably the serializer can be customized such that store.pushPayload will handle the clientId in the payload transparently, right?

---

Your question: “Are there issues with transitioning nested saved objects to an invalid state even in cases where the nested object itself did not have errors?” I don't see any problem with this.

[hjdivad]

How about using a key other than clientId, something like tempId or ephemeralId to signify that it's a temporary value? Nothing major though, clientId totally works — this comment is in the bike-shed category.

Although both tempId and ephemeralId signify that the id is short-lived, neither signify that the id originates from the client. If I had to pick between the two, I'd rather signify the id is client-originated rather than ephemeral. I don't feel strongly about this though.

Is this supposed to work together with EmbeddedRecordsMixin or as an alternative? If it's supposed to work along-side, it would be neat if the logic provided by savedWith could be hidden. I.e. embedded records as defined by the EmbeddedRecordsMixin would always get this behaviour. Or am I missing some obvious drawback with that?

After some discussion, this RFC will be reworked slightly to move savedWith so that it can be entirely an adapter concern. That this will help make EmbeddedRecordsMixin integration smoother is a motivation for this.

With out-of-band, presumably the serializer can be customized such that store.pushPayload will handle the clientId in the payload transparently, right?

Yes. Your serializer will need to ensure that clientId is in the right place and then the store will prioritize updating an existing unpersisted record with that clientId over pushing a new record into the identity map.

[sandstrom]

@hjdivad I was mainly throwing the idea out there (id-naming). If you prefer clientID lets go with that, I'm very happy whichever name it gets 😃

Moving savedWith into an adapter-concern would be awesome! 🚀

[mspisars]

Is there a timeline for this? Can this be brought up for discussion on being added to the timeline?

[robneville73]

is anyone working on this? I might be interested in tackling some of this.

[hjdivad]

@robneville73 nobody is working on it at the moment, but everyone's pretty positive on the idea.

The first step is to rework the rfc (slightly) to make the mechanics entirely an adapter concern.

• move savedWith to snapshot (off DS.Model)
• entangle in snapshot via adapter
• move clientId to the meta portion of a resource to be consistent with jsonapi

[robneville73]

I can fork your rfc and try to do that if you'd like though I'm a little fuzzy on what you mean by entangling in snaphot via adapter.

[hjdivad]

@robneville73

I can fork your rfc and try to do that if you'd like though I'm a little fuzzy on what you mean by entangling in snaphot via adapter.

Basically think through how to change the example when you don't have DS.Model#savedWith.

I imagine the way this would work is that the parent object will be saved and then in its adapter the relevant hook (createRecord, updateRecord) would call belongsTo / hasMany to get the related snapshots to call savedWith.

Something like

// models/order.js
DS.Model.extend({
  items: DS.hasMany('items'),
});

// routes/order.js
Ember.Route.extend({
  actions: {
    saveOrder(order) {
      order.save();
    }
  }
});

// adapters/order.js
DS.Adapter.extend({
  createRecord(store, modelClass, snapshot) {
    snapshot.belongsTo('items').forEach(itemSnapshot => {
      // this would transition `itemSnaphot.record` and entangle `itemSnapshot` with 
      //`snapshot` such that when the order's record transitions away from in flight, 
      // so does the item's record
      itemSnapshot.savedWith(snapshot);
    });
  }
});

Does that make sense?

[hjdivad]

Closing.

Of the three items this RFC is concerned with, two are handled by the merged identifiers rfc.

The remaining item is an API for co-mingling record lifecycles for multiple records with one promise (whether nested saves, or optimistic changes based on knowing the semantics of a particular API) which we'd prefer to do as a separate rfc.