Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

store.findAll doesn't populate meta property when promise resolves #5337

Closed
mehulkar opened this issue Jan 25, 2018 · 1 comment
Closed

store.findAll doesn't populate meta property when promise resolves #5337

mehulkar opened this issue Jan 25, 2018 · 1 comment

Comments

@mehulkar
Copy link

mehulkar commented Jan 25, 2018

Issue

In 2.18.0, store.findAll does not set the meta property on the resolved RecordArray.

Example

I wasn't sure how best to create a live example of this without a backend, but here's an example:

response:

{
  "data": [
    { "id": "1", "type": "post", "attributes": {  "name": "Foo Bar" } },
    { "id": "2", "type": "post", "attributes": { "name": "Foo Bar" } }
  ],
  "included": [],
  "links": {},
  "meta": {
    "someProp": "a meta prop!",
    "anotherProp": 42
  }
}
// meta is set
store.query('post', {}).then(result => result.get('meta'))

// meta is not set
store.findAll('post').then(result => result.get('meta')) 

Cause

The query method sets up the RecordArray with a RecordArrayManager which sets the meta property:

https://github.com/emberjs/data/blob/v2.18.0/addon/-private/system/store/finders.js#L166

whereas the findAll method does not use the manager.

Failing test case

test("meta is proxied correctly on the PromiseArray", function(assert) {
  let defered = RSVP.defer();

  env.registry.register('adapter:person', DS.Adapter.extend({
    findAll(store, type, query) {
      return defered.promise;
    }
  }));

  let result;
  run(function() {
    result = store.findAll('person');
  });

  assert.notOk(result.get('meta.foo'), 'precond: meta is not yet set');

  run(function() {
    defered.resolve({ data: [], meta: { foo: 'bar' } });
  });

  assert.equal(result.get('meta.foo'), 'bar');
});

I just copied the one for query and replaced it with findAll: https://github.com/emberjs/data/blob/v2.18.0/tests/integration/store/query-test.js#L28-L49

@runspired
Copy link
Contributor

Closing as a duplicate of #2905

That said, it is difficult for us to provide support for meta on the RecordArray returned by findAll for the following reasons:

  • findAll('foo') returns the same RecordArray as peekAll('foo'). Having meta for an unknown request would be confusing for folks using peekAll.
  • findAll('foo') may or may not hit network, and may or may not do so as a background request, it is uncertain what meta would refer to in this scenario.
  • findAll('foo') contains all available records from all requests ever made as well as anything created client side, not just from the single request it may also make itself. It is uncertain how any meta object from a single specific request would be applicable to this array.

The above said, we are committed to building better APIs with first-class json-api support in the near future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants