Proper separation between the serializer class vs. the serializer instance about fast_jsonapi HOT 9 OPEN

@christophersansone I like the idea of having a clear guideline about what goes into the serializer class and what goes into the serializer instance. I wonder if we can provide backward compatibility to the current way of doing things so that users of the gem can upgrade without changing things around a lot.

from fast_jsonapi.

@shishirmk Yeah I was thinking about the upgrade path too. I think we can potentially offer backwards compatibility for the short-term and provide deprecation warnings. But it seems like the sooner we implement this, the fewer people will have to change.

I'm thinking we wait until after next week's release so we don't hold it up, and then tackle this right afterwards. Does that seem reasonable?

from fast_jsonapi.

I attempted to refactor fast_jsonapi because of this exact reason: I was confused with abstraction distribution across instance-class levels. But it turned out to be a complete rewrite, the result can be checked out here: https://github.com/youroff/alt_jsonapi

Core idea is that class is the place where serializer is configured and then using this configuration and fields and include params, it builds a tree of serializer instances that in its turn is used for serialization itself. It already supports custom attributes, polymorphic relationships etc.

Eventually it can be pulled in to fast_jsonapi if maintainers decide to do so. However that are number of architectural decisions that are different with fast_jsonapi and might be incompatible.

• Inheritance of serializers is explicitly forbidden, because it's reserved for PolymorphicSerializer. Though it's technically possible to make so that regular serializers were supporting inheritance.
• It doesn't know anything about ActiveRecord and its conventions (like relation_id / _ids), so even without inclusion of relationships, they must be preloaded to build id hashes. The reasons are described in documentation.
• There is no internal caching, and honestly I don't see the right place and way to put it there.

So please try it and let me know if you find any issues and/or get ideas how to make it better! :)

from fast_jsonapi.

@shishirmk I feel like we are starting to be blocked by the amount of divergence between dev and master. I see there is a v1.1 milestone, but I'm wondering if we can help speed up development and issue resolution if we release a smaller version bump like 1.0.18. If there are no breaking changes between dev and master, can we do this? If there are breaking changes, can we consider reducing the scope of v1.1 and push some of the less complete features to a future version?

from fast_jsonapi.

@christophersansone yes we are. I think we can release v1.1 with what we have in dev now. It is not ideal but i think it's a good next step. I would like to document some known issues before we do the release.

There is a minor breaking change hence i would like to bump it to v1.1.0. We dont support use_hyphen anymore. It is being replaced by key_transform.

@mrryanjohnston is testing parts of the dev branch. I would like to test the branch on some of our internal projects. Once it looks good. I am planning to release v1.1.0

from fast_jsonapi.

@shishirmk Awesome, thanks. If use_hyphen is the only breaking change, would it be better to deprecate it rather than remove it all together? The implementation can just call the new key transform method and serve a deprecation warning. It would be simple to do, and we can remove it in a future release that will have more significant breaking changes that we can't easily deprecate.

For upcoming releases, it will help to tag PRs with "breaking change" when appropriate, and perhaps require the author to update a CHANGELOG to document the code changes the users need to implement upon release.

from fast_jsonapi.

Ya i agree lets just add support for use_hyphen and we can just release 1.0.18

from fast_jsonapi.

@shishirmk Thanks for getting 1.1 out the door! It seems like this is probably the next big blocker to address before some of the other features can be implemented, right? Want me to give this a shot this week?

from fast_jsonapi.

@christophersansone ya i think after we address #79. This would be a good thing to to tackle.

from fast_jsonapi.