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

1.13 application serializer doesn't default to RESTSerializer #3404

Closed
bcardarella opened this issue Jun 21, 2015 · 17 comments
Closed

1.13 application serializer doesn't default to RESTSerializer #3404

bcardarella opened this issue Jun 21, 2015 · 17 comments

Comments

@bcardarella
Copy link
Contributor

it appears to be defaulting to JSONAPISerializer. This should probably default to RESTSerializer and throw a deprecation warning similar to defaultAdapter

@igorT
Copy link
Member

igorT commented Jun 21, 2015

@wecc

@wecc
Copy link
Contributor

wecc commented Jun 21, 2015

The default serializer is RESTSerializer but it automatically opts into the new Serializer API.

@jcbvm
Copy link

jcbvm commented Jun 22, 2015

Does opt-in mean that it is using the new API or not, I get a lot of errors which seem to come from models which don't have a serializer defined (so a default serializer will be created with opt-in for the new API), so I suspect that it tries to use the new API for a payload which is not structured the new API way.

@wecc
Copy link
Contributor

wecc commented Jun 22, 2015

@jcbvm Opting into the new Serializer API basically means that the store calls serializer.normalizeResponse instead of serializer.extract and expects to get a JSON-API Document back from the serializer.

The built-in serializers supports both the old and the new Serializer API out of the box and automatically returns a JSON-API Document if they are using the new Serializer API. If you haven't defined a serializer it will automatically opt into the new Serializer API since we know that you haven't done any customizations. It should just work.

The format of your server response should not have to change when using RESTSerializer or JSONSerializer, regardless if the serializer uses the old or new Serializer API. It's all about how the store interacts with the serializer.

If you don't have any serializers defined and still get errors, please open an issue as that is most likely a bug.

@jcbvm
Copy link

jcbvm commented Jun 23, 2015

Thanks for the clear response, I'll take a look in my issues to see what's really going wrong.

@bcardarella
Copy link
Contributor Author

@wecc opting into the new serializer API for the 1.x series sounds like a mistake. Why change the way the library has been working for everyone else for the past few months and not hold this decision off until 2.x?

@wecc
Copy link
Contributor

wecc commented Jun 23, 2015

@bcardarella The RESTSerializer, JSONSerializer and ActiveModelSerializer all supports both the old and the new Serializer API. If you haven't done any customizations, as in haven't defined any custom serializers, the serializers automatically opts into the new Serializer API to prevent any deprecations from showing up. This should be fully backwards compatible and no changes at all should be required by you.

If you do have custom serializers we cannot automatically opt in as that might break (not necessarily, but probably), that's why custom serializers stay on the old Serializer API until you manually opt in using the isNewSerializerAPI flag.

The old Serializer API will be gone in 2.0, so we have to deprecate it now. We don't want to deprecate anything without having an alternative route, that's why we support both the old and the new Serializer API in 1.13.

Are you experiencing issues without custom serializers? If so, it should be considered a bug and it would be great if you can report it so it can be fixed. I'm also available on Slack and I'd be happy to help.

@bcardarella
Copy link
Contributor Author

I'm afraid this is the point, the expected behavior up until this release was not the new serializer. Opting into the "new way" automatically for everyone in the 1.x series breaks everyone's apps. Not knowing about the flag to set can mean a lot of people are going to spend a lot of time head desking trying to figure out what is going on. It would have been best to have at least one single stable release that was consistent with the beta series then move over to this flag. It's too far gone at this point but it feels like the trigger on this was pulled way too soon.

@wecc
Copy link
Contributor

wecc commented Jun 23, 2015

Opting into the "new way" automatically for everyone in the 1.x series breaks everyone's apps

This is not what we're doing. We're not opting into the new Serializer API automatically for everyone. We only do it when you're using default, built-in, standard, non-customized serializers. We have been working hard to maintain backwards compatibility and regardless if opting in or not your app shouldn't break.

In what way does your app break?

@bcardarella
Copy link
Contributor Author

We only do it when you're using default, built-in, standard, non-customized serializers

this is the problem, there are cases where the backend APIs have been customized to fit Ember's default serializer's needs rather than the other way around.

@wecc
Copy link
Contributor

wecc commented Jun 23, 2015

The new Serializer API has nothing to do with your backend API or what your server payloads looks like. It's only about the communication between the store and the serializer.

Take the RESTSerializer for example:

Using the old Serializer API

  1. Your server returns a payload that has been customized to fit the default RESTSerializer
  2. The RESTSerializer normalizes your payload to the old Ember Data internal format and returns it to the store
  3. The store knows you're on the old Serializer API and converts the old internal format to JSON-API
  4. The store knows how to deal with JSON-API

Using the new Serializer API

  1. Your server returns a payload that has been customized to fit the default RESTSerializer
  2. The RESTSerializer normalizes your payload to JSON-API and returns it to the store
  3. The store knows how to deal with JSON-API

@jcbvm
Copy link

jcbvm commented Jun 23, 2015

Refering to point 3 of the first list, does this mean the store looks at the retrieved data? Or does it look if a custom serializer is used? It the latter is the case, it might break existing apps.

@wecc
Copy link
Contributor

wecc commented Jun 23, 2015

@jcbvm the store looks at the isNewSerializerAPI flag of the serializer to tell if it should call the old extract or new normalizeResponse method. If it calls extract it knows it will get the old internal format back and converts that to JSON-API before continuing. If it calls normalizeResponse it expects JSON-API back and can continue without any conversion.

So, the store knows how to convert the old internal format to JSON-API and the built-in serializers are able to return either the old format or JSON-API.

@jcbvm
Copy link

jcbvm commented Jun 23, 2015

Ok so if I understand correctly, if I do not define a custom serializer, the flag isNewSerializerAPI will be true. Like @bcardarella said, if the server is returning a payload the old fashion way it will break the app, this is probably the case in my related issue #3424

@wecc
Copy link
Contributor

wecc commented Jun 23, 2015

@jcbvm if you do not define a custom serializer, the flag isNewSerializerAPI will be set to true automatically, confirm. But it should not break your app, the serializer will still expect the same identical payload from your server regardless of the old or new Serializer API.

@wecc
Copy link
Contributor

wecc commented Jun 29, 2015

I don't think there are any specific issues related to the new Serializer API that are breaking apps so I'm closing this. Please feel free to reopen if I'm mistaken.

@wecc wecc closed this as completed Jun 29, 2015
@miketervela
Copy link

miketervela commented Nov 10, 2017

I know this is ancient and there probably not a lot of people still upgrading to 1.13, but here's what I discovered:
everything loaded in the same pushpayload must use the same API.
So if in 1.12 you have several rest objects sideloaded together, say post with sideloaded comment and author, but only needed to override the serializer for 'post', then you'd have a single serializer file: post.js.
When you upgrade to 1.13, your old post serializer will automatically continue using the old API but comment and author will use serializers with the new API and when you receive a post payload with comment and author sideloaded, the push will fail - so far usually with
You must include an `id` for comment in an object passed to 'push'

So you then need to create an empty serializer for comment and author without overriding isNewSerializerAPI

Then if you sideload other objects with author and comment, you'll need to give those sideloaded objects empty rest serializers as well.

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

5 participants