search

beauby / jsonapi-datastore

JavaScript client-side JSON API data handling made easy.
MIT License
195 stars 40 forks source link

adding meta data support #10

Closed jprincipe closed 8 years ago

jprincipe commented 8 years ago

this PR adds support for processing meta data from the JSON payload. For backwards compatibility I left the sync() method the same and added a new syncWithMeta() method that returns both the data and meta data at the same time. The key for meta data is defaulted to meta but it can be overridden using the setMetaKey method.

beauby commented 8 years ago

Very useful PR indeed, although I believe the JSON API spec does not allow for a "custom meta key", so the setMetaKey kind of bothers me. Thoughts?

jprincipe commented 8 years ago

I agree based on the spec. I only added it because active_model_serializers allows for customizing the key

beauby commented 8 years ago

Actually AMS allows this only for the Json serializer, not for the JsonApi one. I think we can remove it for now, and if there's a strong need for it (which I doubt) we'll add it back.

jprincipe commented 8 years ago

Just looked at the ams source and meta_key is defined in the base adapter. I see that you have a change in ams master around the meta_key. Is that code in rc3?

https://github.com/rails-api/active_model_serializers/blob/6018ef16c41c441aeb1621852c6f58571b80e656/lib/active_model/serializer/adapter/base.rb#L43

I tested this out locally on rc3 and sure enough if I set meta_key in the render, the meta data is returned with the custom key

beauby commented 8 years ago

Yeah, but this is just a byproduct of shared code between adapters. AMS does not officially support custom meta key, and I'll probably get rid of it when I'm done with deserialization and some other stuff.

jprincipe commented 8 years ago

@beauby I removed the meta_key from the code and defaulted the meta response to null if the meta key doesn't exist

beauby commented 8 years ago

LGTM – If you could just fix the docs to specify which meta property it supports, that'd be great! Thanks again :+1: