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

Serializer issue with embedded relationships. #3424

Closed
jcbvm opened this issue Jun 23, 2015 · 10 comments
Closed

Serializer issue with embedded relationships. #3424

jcbvm opened this issue Jun 23, 2015 · 10 comments

Comments

@jcbvm
Copy link

jcbvm commented Jun 23, 2015

Got a lot of you must include an 'id' for some-model in an object passed to 'push' errors since I upgraded to 1.13.4. Tracked it down and it seems to come from my embedded relationships. When I do not define a custom serializer for the embedded relationship, above error is thrown. When I do create a custom serializer (just an empty RESTSerializer), above error disappears.

@wecc
Copy link
Contributor

wecc commented Jun 23, 2015

Thanks for reporting! Would you be able to reproduce the error in a JSBin?

@jcbvm
Copy link
Author

jcbvm commented Jun 23, 2015

Will try to later this afternoon.

@wecc
Copy link
Contributor

wecc commented Jun 23, 2015

Here's a pre-configured JSBin using Ember 1.13.2 and Ember Data 1.13.4 without any deprecation warnings you can use: http://emberjs.jsbin.com/zotuta/edit?html,js,output

@jcbvm
Copy link
Author

jcbvm commented Jun 23, 2015

Here's a JSBin reproducing the error http://emberjs.jsbin.com/kubupaluma/edit?html,js,output

@wecc
Copy link
Contributor

wecc commented Jun 23, 2015

Thanks @jcbvm! This seem to be a bug where your serializer:book extracts it's embedded author and looks up the serializer:author to normalize the author's data. And because serializer:book is defined in your app it's not using the new Serializer API, but serializer:author is looked up and automatically opted in to the new Serializer API... so two serializers are working together on two different APIs.

I'm not sure how to solve this but it should be fixed.

Until then you can either have your serializer:book opt into the new Serializer API by setting isNewSerializerAPI: true or just define serializer:author to make sure it too uses the old Serializer API.

@jcbvm
Copy link
Author

jcbvm commented Jun 23, 2015

Thanks for looking into, makes some sense now. Currently I'm working around this by defining an empty serializer, so using the old API.

@igorT
Copy link
Member

igorT commented Jun 27, 2015

@wecc this is actually tricky, we should think if there is a way to make sure you always use new or old serializer throughout the request

@wecc
Copy link
Contributor

wecc commented Jun 28, 2015

@igorT there's an assert in RESTSerializer that doesn't solve this particular problem, but might at least give some information about what's happening, see https://github.com/emberjs/data/blob/release/packages/ember-data/lib/serializers/rest-serializer.js#L218

We could implement this assert in multiple places (where we do serializerFor within serializers, practically)

@bmac
Copy link
Member

bmac commented Jun 29, 2015

We could implement this assert in multiple places (where we do serializerFor within serializers, practically)

I think this sounds like a great idea.

@wecc
Copy link
Contributor

wecc commented Jul 7, 2015

Closing since #3511 has been merged.

@wecc wecc closed this as completed Jul 7, 2015
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

4 participants