RESTSerializer.pushPayload does not allow sideloading of records

[amk221]

Given a payload like:

{
     posts: [{}]
     _posts: [{}, {}]
}

I get this warning:
WARNING: Encountered "_posts" in payload, but no model was found for model name "-post" (resolved model name using blah@serializer:post:.typeForRoot("_posts"))

1.0.0-beta.18

[fivetanley]

You should remove the underscore in the payload or override modelNameFromPayloadKey (we need to update the message it looks like).

Ember.String.pluralize('foo') == 'foos'.

[amk221]

Quote from this page:
https://github.com/emberjs/data/blob/master/CHANGELOG.md#ember-data-100-beta3-september-28-2013

"Support primary and sideloaded data of the same type to be returned from array lookups (via posts and _posts)."

@fivetanley this looks like a regression?

[wecc]

@amk221 You are correct. Sideloading secondary records of the same type as primary is supported in RESTSerializers extractArray using _type as key.

However, I'm unable to reproduce the error, see http://emberjs.jsbin.com/riyevivoxi/1/edit

Would you be able to provide a JSBin demonstrating the error?

[amk221]

Ah, you're right

But, put that exact same code in a fresh ember-cli project (0.2.5) and it doesn't work

[amk221]

I closed this a while ago, but realised it's still happening

In a jsbin/fiddle/twiddle:

store.modelFactoryFor('post') // function .Post
store.modelFactoryFor('_post') // function .Post

In an ember-cli app

store.modelFactoryFor('post') // function my-app@model:post:
store.modelFactoryFor('_post') // undefined

[wecc]

This seem unrelated since the RESTSerializer removes the prefixing _ when sideloading resources of the same type as the primary.

When does store.modelFactoryFor('_post') occur?

[amk221]

for (var prop in payload) {  // prop = '_posts'
      var modelName = this.modelNameFromPayloadKey(prop); // modelName = '-post'
      if (!store.modelFactoryFor(modelName)) {  // modelFactoryFor('-post')
        Ember.warn(...
// ...

It's checking for the dash rather than an underscore?

[wecc]

can you link to that code?

[amk221]

Ah, sure: rest-serializer.js#pushPayload

[wecc]

Ah, pushPayload()... hm, seems the behavior in pushPayload() and normalizeResponse() is a bit different. I can see why you'd expect them both to accept the same payloads even though it doesn't really matter to have records of the same type in a separate key for pushPayload() since it doesn't return anything.

[amk221]

It was the same reason I mentioned in Slack earlier - if you manually pushPayload it warns that meta is present (tries to load a model named metum!). But it doesn't warn if you use findAll.

Essentially I was expecting pushPayload to be able to accept the exact same structure that would have been loaded had I used findAll.

[wecc]

We discussed this issue at today's meeting and I'll have a look at this and see if/how we can implement this in a backwards compatible way. Should probably not be that hard.

Essentially I was expecting pushPayload to be able to accept the exact same structure that would have been loaded had I used findAll.

👍

[wecc]

@pangratz Would this be covered by emberjs/rfcs#161?

[pangratz]

That RFC doesn't explicitly handle the inconsistencies between pushPayload and normalizeResponse in the rest serializer and also doesn't explicitly state the behavioral differences between normalizeResponse and pushPayload.

So I tend to think that this is a bug in rest serializer, which could be fixed in the short term by adding logic for keys starting with _ in pushPayload. In the long term we should evaluate merging the logic within normalizeResponse and pushPayload of the rest serializer, like we already do in the json-api serializer.

[amk221]

Any update on this, it's been a long while

[runspired]

@amk221 I would recommend against using pushPayload in general. If you have a moment to work on an RFC: #8815

[runspired]

Closing, as making changes to pushPayload is not something we desire to do, and because pushPayload requires too much "special sauce" and "magical format knowledge" as it is.

[amk221]

Oh dear. Does this mean I should be phasing this out of my app? If so, what would I replace it with? Thanks

…

On Fri, 16 Nov 2018 at 22:04, Chris Thoburn ***@***.***> wrote: Closing, as making changes to pushPayload is not something we desire to do, and because pushPayload requires too much "special sauce" and "magical format knowledge" as it is. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3090 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAXUbQoap1lo_olWbzFAy7dOk52JEq3Bks5uvzZIgaJpZM4ElJby> .