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

Can't return null to queryRecord anymore #4310

Closed
danconnell opened this issue Apr 7, 2016 · 9 comments
Closed

Can't return null to queryRecord anymore #4310

danconnell opened this issue Apr 7, 2016 · 9 comments
Labels
🏷️ bug This PR primarily fixes a reported issue

Comments

@danconnell
Copy link

Previously on Ember Data 2.3.3, you could have you API return null to a call made by queryRecord. Now, on Ember Data 2.4.3, you get this error if you try to do that:

Assertion Failed: You made aqueryRecordrequest for undefinedrecords, with query[object Object], but the adapter's response did not have any data Error: Assertion Failed: You made aqueryRecordrequest for undefinedrecords, with query[object Object], but the adapter's response did not have any data

Was this an intended change? I understand that this might not be a bug, but I also don't see this change mentioned in the changelog.

Just thought I should point it out, since it did break something for me.

@pangratz
Copy link
Member

pangratz commented Apr 7, 2016

Thanks for reporting this @danconnell. What adapter are you using? Can you provide an ember-twiddle, demonstrating the issue? You can use this twiddle as a start, it uses the json-api adapter. Thanks!

@pangratz
Copy link
Member

pangratz commented Apr 7, 2016

Ah now I can reproduce the issue you're having. I had it wrong, and in the twiddle { data: null } is returned which is not the same thing you return I assume.


The issue you're having has been fixed in #4256, which is available in 2.5.0-beta.3. Can you try out that version and confirm that your app is working again? Edit: the linked issue #4256 is unrelated to this one here.

@danconnell
Copy link
Author

I'm using the rest adapter.

Tested on 2.5.0-beta.3 and it's still broken. Here's a twiddle using 2.5.0-beta.3.

@pangratz
Copy link
Member

pangratz commented Apr 7, 2016

Looks like this is a regression. There is no assert throwing in v2.3.3 but in v2.4.3 there is.

The assertion has been added in #3916, because at that time a not very descriptive error You must include an 'id' for undefined in an object passed to 'push' would be thrown if the payload was empty. Maybe this is already better handled in the meantime...

@pangratz
Copy link
Member

This has been discussed in the team meeting and the assertion for queryRecord should be removed and the old behavior should be restored. This will be tackled in the course of #4300.

@danconnell
Copy link
Author

Sounds good. Thanks for the update :)

@BissyQA
Copy link

BissyQA commented Apr 17, 2016

same error Assertion Failed: You made a queryRecord request for undefinedrecords, with query `[object Object]. Does the issue fixed in 4300? . Is this alternate way to fix the error ?

because at that time a not very descriptive error You must include an 'id' for undefined in an object passed to 'push' would be thrown if the payload was empty.

@pangratz
Copy link
Member

pangratz commented Apr 19, 2016

@BissyQA yes, this is addressed in #4300. Until the PR is merged and a new release is cut, you can work around this by ensuring that you don't return an empty object as payload from the adapter, as shown in this ember-twiddle:

import DS from "ember-data";

export default DS.RESTAdapter.extend({
  queryRecord() {
    return this._super(...arguments).then(function(payload) {
      // workaround so returned payload is not empty, remove
      // once https://github.com/emberjs/data/pull/4300
      // is merged and released
      payload._unusedKeySoPayloadIsNotEmpty = {};

      return payload;
    });
  }
});

@pangratz
Copy link
Member

pangratz commented May 4, 2016

I am going to close this issue since #4300 has been merged and the regression has been fixed in that PR.

@pangratz pangratz closed this as completed May 4, 2016
@runspired runspired added 🏷️ bug This PR primarily fixes a reported issue and removed Regression labels Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bug This PR primarily fixes a reported issue
Projects
None yet
Development

No branches or pull requests

4 participants