rfcs
rfcs copied to clipboard
[DEPRECATION] deprecate store.pushPayload and serializer.pushPayload
Alternatives:
- Use
store.pushfor already normalized payloads - Use
store.serializerForandserializer.normalizeResponseto normalize payloads before usingstore.push. - Some previous discussion https://github.com/emberjs/data/issues/4213#issuecomment-413370235
- Some examples of how to work without this method: https://github.com/emberjs/data/pull/4110#issuecomment-417391930
That probably requires a little bit more of explanation, here is a use-case that I have:
I use pushPayload a lot, as I do interact with API through simple requests, get JSON API in return, and push to ember-data when I know that I will work with those records. As I see it, ember-data should be aware of how to hydrate supported format (json-api) into the models. It just fills weird, that for sort-of standard operations, something custom is required. Serializers are part of the ember-data, as well as JSON API format itself.
Can we get a little bit of context, why this 4 lines for pushPayload can't stay in ember-data?
@RuslanZavacky
As I see it, ember-data should be aware of how to hydrate supported format (json-api) into the models.
This is exactly how store.push works and is what it is for.
Serializers are part of the ember-data
If you are already in json-api format, there's no reason for a serializer. Additionally, serializers may not remain in ember-data much longer in their current form. We're working on a more robust primitive to take their place.
why this 4 lines for pushPayload can't stay in ember-data?
It isn't 4 lines. It's much more once you consider the corresponding serializer code and a terrible amount more still once you consider that app engineers must align their payloads to a hidden, highly inefficient, non-json-api format for the method to even work. Furthermore, an unfortunate design mistake means that for most applications, the corresponding serializer hook has zero context on the payload being normalized (it does not receive the modelName included in the call to store.pushPayload).
It isn't 4 lines. It's much more once you consider the corresponding serializer code and a terrible amount more still once you consider that app engineers must align their payloads to a hidden, highly inefficient, non-json-api format for the method to even work. Furthermore, an unfortunate design mistake means that for most applications, the corresponding serializer hook has zero context on the payload being normalized (it does not receive the modelName included in the call to store.pushPayload).
Can this be added to the RFC? What's this unfortunate design mistake? Why is it important for the serializer hook to have context around the payload being normalized?
Considering my own confusion around these two methods, as well as watching the comments around this stuff recently, I would propose updating docs to make it clearer that store.push() is for payloads already in json-api format, and if your json is not in json-api format, then store.pushPayload() + custom serializer is (current-day) how you should push the json into the store, but going forward you will need to convert your json payload into something that is json-api compliant & use store.push().
Currently, the docs section titled Store createRecord() vs. push() vs. pushPayload() reads:
push is used to notify Ember Data's store of new or updated records that exist in the backend. This will return a record in the loaded.saved state. The primary use-case for store#push is to notify Ember Data about record updates (full or partial) that happen outside of the normal adapter methods (for example SSE or Web Sockets).
pushPayload is a convenience wrapper for store#push that will deserialize payloads if the Serializer implements a pushPayload method.
Nowhere does either paragraph mention json-api vs non-json-api. I think that simple distinction would have helped me a lot.
@runspired I've been trying to convert our pushPayload calls to use push and have encountered an issue.
It seems that store.push() expects type: ${modelName} to be singularized, even though model.serialize() produces a type that is pluralized (and it appears that our API pluralizes model names by default).
When you have an application that uses the JSONAPIAdapter, shouldn't store.push(model.serialize({includeId: true})) not produce an error?
I guess that json-api spec is "agnostic about inflection rules"... which implies both singularized & pluralized type strings should work with store.push. Right?
I was playing around on ember-twiddle and for the life of me I can't figure out why the serialized representation is different in testPush vs testPushPayload
@sdhull feel free to ping me on slack or discord to avoid sidetracking the conversation here; however, the short answer to your troubles is that serialize is that the store expects singular, dasherized types and camelCase members; however, as you noted the spec is agnostic to whether type is pluralized or not and dasherized or not. There's an assumption (largely a hold-over from the rails active-record days) that APIs largely work with pluralized types. Something we seek to do soon in ember-data is make it agnostic: what you give us is what you get. You must be consistent (and we will enforce consistency), but there won't be any other restrictions.
One of the downsides of the existing serializer implementation is that folks have never come to realize the criteria for what the store actually needs, resulting in folks aligning to the serializer spec instead of the store spec. Folks using json-api shouldn't require a normalization method at all if aligned, but unfortunately they haven't known what to align to.
That said, even for the simple case of fixing types, authoring your own serializer is often cleaner and many orders of magnitude more maintainable and performant. I left a comment with a demo of this here: https://github.com/emberjs/data/pull/4110#issuecomment-417391930
ping me on slack
discord? :trollface: https://discordapp.com/invite/zT3asNS
I'm closing this due to inactivity. This doesn't mean that the idea present here is invalid, but that, unfortunately, nobody has taken the effort to spearhead it and bring it to completion. Please feel free to advocate for it if you believe that this is still worth pursuing. Thanks!
we're still planning on tackling this