Data: Accessing `include` query parameter with `coalesceFIndRequests = true`

[buschtoens]

When coalesceFindRequests = true the include option is not passed through to the snapshot. This means, that it is impossible to access this option in the findMany method:

export default JSONAPIAdapter.extend({
  coalesceFindRequests: true,

  findMany(store, type, ids, snapshots) {
    console.log(snapshots[0].include); // undefined
    return this._super(...arguments);
  }
});

This is because of these lines in addon/-private/system/store.js

_flushPendingFetchForType(pendingFetchItems, modelName) {
  // ...
  let snapshots = new Array(totalItems);
  for (let i = 0; i < totalItems; i++) {
    snapshots[i] = internalModels[i].createSnapshot();
  }
  // ...
}

and addon/-private/system/store/finders.js

export function _findMany(adapter, store, modelName, ids, internalModels) {
  let snapshots = Ember.A(internalModels).invoke('createSnapshot');
  // ...
}

Two questions arise:

1) Why are the pendingItem.options not passed to createSnapshot() in store.js?
2) Why are new snapshots created in _findMany in finders.js?

I made some naive changes to both files and was able to pass the options to the createSnapshot() method in store.js and then use these snapshots in finders.js.

let snapshots = new Array(totalItems);
for (let i = 0; i < totalItems; i++) {
  let internalModel = internalModels[i];
  let pendingItem = seeking[internalModel.id];
  snapshots[i] = internalModel.createSnapshot(pendingItem.options);
}

export function _findMany(adapter, store, modelName, ids, internalModels, snapshots) {
  snapshots = snapshots || Ember.A(internalModels).invoke('createSnapshot');
  // ...
}

I don't know much about the inner workings of Ember Data and how this might interfere with other parts of the library, but these changes introduced no apparent bugs / errors in our existing relatively complicated app.

I assume that this is just an oversight, as passing include for coalesced find requests isn't really exactly defined behavior.

[buschtoens]

In store#_flushPendingFetchForType(pendingFetchItems, modelName) it says:

      // TODO: Improve records => snapshots => records => snapshots
      //
      // We want to provide records to all store methods and snapshots to all
      // adapter methods. To make sure we're doing that we're providing an array
      // of snapshots to adapter.groupRecordsForFindMany(), which in turn will
      // return grouped snapshots instead of grouped records.
      //
      // But since the _findMany() finder is a store method we need to get the
      // records from the grouped snapshots even though the _findMany() finder
      // will once again convert the records to snapshots for adapter.findMany()

How is that suppsoed to be interpreted? Is the conversion from records => snapshots => records => snapshots desired or should it be replace? Because that is the reason (or part of it) that include gets swallowed.

[veelci]

Any updates on this? I'm running into this scenario as well.

[logmannn]

If you are using 'includes' on your findRecord's coalesceFindRequests=true will effectively give you the opposite behavior you desire. It will group the request, then fire off sep requests for each individual include.

If you had 30 find records and each one needs 3 included records, you get 90 requests using coalesceFindRequests=true. Instead of the 1 request that this is supposed to coalesce (or the 30 that you would have seen setting coalesceFindRequests=false.

@runspired Thus! I believe this issue should be marked as a bug and not a 'improvement'.

[runspired]

@logmannn a reproduction/fix is welcome. Both for the original threading PR that got abandoned and for the reported bug of this triggering extra requests instead of fewer.