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

queryRecord throws when the response is empty #3890

Closed
XrXr opened this issue Oct 26, 2015 · 10 comments
Closed

queryRecord throws when the response is empty #3890

XrXr opened this issue Oct 26, 2015 · 10 comments

Comments

@XrXr
Copy link
Contributor

XrXr commented Oct 26, 2015

http://emberjs.jsbin.com/fekufi/1/edit?js,console

Error: Assertion Failed: You must include an 'id' for undefined in an object passed to 'push'

If the payload for authors is changed to null, the same error happens.

This use to just resolve with undefined in ED 1.3.x, so I assumed it's a convenience feature.

I asked about this on Slack, the consensus seems to be that the promise should reject like a 404.

@XrXr
Copy link
Contributor Author

XrXr commented Oct 26, 2015

Here is a bin for JSONAPIAdapter

http://emberjs.jsbin.com/fekufi/edit?js,console

SyntaxError: Unexpected end of input
at Object.parse (native)
at x.extend.parseJSON

@sescobb27
Copy link

@XrXr http://jsonapi.org/format/#fetching in JSONAPI payload can't be empty. you SHOULD return

{
  "data": []
}

or

{
  "data": null
}

@jherdman
Copy link

I ran into this too. The JSON payload is intentionally empty though.

Here's another contrived example: http://emberjs.jsbin.com/sikivanomi/edit?html,js,output. Simply put, my query found nothing on the back end. This was a valid response in ED 1.13, I don't see why my API response wouldn't be valid now.

@sescobb27
Copy link

This works as expected

// Returns books embedded in authors
$.mockjax({
  type: 'GET',
  url: '/authors',
  status: '204',
  dataType: 'json',
  responseText: '{"data": []}'
});

@jherdman
Copy link

I think the issue is more accurately stated as "queryRecord shits the bed with REST Adapter when empty response returned"

@sescobb27
Copy link

@jherdman I think is because you are querying just a record, but you are returning an array (an empty one)

@jherdman
Copy link

Indeed, but that's how this jazz works. Peep this:

if (isSingle) {
/*jshint loopfunc:true*/
data.forEach((resource) => {
/*
Figures out if this is the primary record or not.
It's either:
1. The record with the same ID as the original request
2. If it's a newly created record without an ID, the first record
in the array
*/
var isUpdatedRecord = isPrimary && coerceId(resource.id) === id;
var isFirstCreatedRecord = isPrimary && !id && !documentHash.data;
if (isFirstCreatedRecord || isUpdatedRecord) {
documentHash.data = resource;
} else {
documentHash.included.push(resource);
}
});
} else {

This is the meat and potatoes of how the REST Adapter goes about figuring out which record to pluck out of a response when you're querying for a record. Indeed the adapter is expecting an array response. The trick seems to be further down in the stack. After this your normalized response look like this: { data: null, included: [] }. The data: null bit causes all sorts of barfage and sadness.

@sescobb27
Copy link

but that's because REST adapter serialize plain JSON payload into JSON API compliant payload in order to let ED load it into the store

@wecc
Copy link
Contributor

wecc commented Oct 31, 2015

Seems related to #3790

@pangratz
Copy link
Member

I am going to close this issue as the behavior of queryRecord has been overhauled in #4300. Please see that PR for more context but basically returning { data: null } or { data: [] } for a queryRecord using the rest adapter/serializer now resolves the promise with null.

Please note that returning an array for the primary data of a queryRecord is deprecated, as this method should only be used for use cases where a single record is returned. In all other cases query should be used instead.

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

5 participants