search

vt-elixir / ja_serializer

JSONAPI.org Serialization in Elixir.
Other
640 stars 148 forks source link

Is it possible to use ja_serializer with a newly generated Phoenix 1.7.7 app? #346

Open theirishpenguin opened 1 year ago

theirishpenguin commented 1 year ago

The Problem

A newly generated Phoenix 1.7.7 app does not have Phoenix.View but ja_serializer depends on Phoenix.View.

From the Phoenix 1.7.7 release announcement...

image

I've tried a few things but I get a variety of errors no matter what I try.

Is it correct to say that the new Phoenix.Template approach of Phoenix 1.7.7 will not currently work with the latest version of ja_serializer? Are there any plans to make ja-serializer integrate with Phoenix 1.7.7?

Potential Temporary Workaround

I notice in the Phoenix 1.7.7 upgrade guide that this might be a possibility...

image

... but going forward that workaround (assuming it even works) would be problematic. It would commit someone, for example when Phoenix 1.9.0 is released, to create a Phoenix 1.6.X and then keep upgrading to 1.9.0.

Any thoughts?

theirishpenguin commented 1 year ago

P.S. This other issue might suggest a solution (ie. removing dependency on Phoenix.View) but I don't know enough about ja_serializer internals to be able to say if this is the right direction for a Phoenix 1.7.7 integration.

beerlington commented 1 year ago

I'm currently using JaSerializer on a Phoenix 1.7.3 app and haven't run into any issues. Is there something specific about 1.7.7 that breaks it? I haven't had a chance to try it with the newest version, but I would be surprised if there was a backwards incompatibility.

The issue you posted to in your second comment is more about removing JaSerializer.PhoenixView, not a dependency on Phoenix.View. Our app is still using JaSerializer.PhoenixView, so I would expect that to also work for 1.7.7.

What errors are you running into?

theirishpenguin commented 1 year ago

@beerlington Ahhh, thanks for the clarification on Phoenix.View vs PhoenixView. I will create a clean example repo later (with a 1.6.X and 1.7.X example) and give more details. Thanks for the quick reply.

theirishpenguin commented 1 year ago

@beerlington Okay, I'm back with two example repos...

All you need to do in either repo is run mix test to see the difference. The error in the example_17x_app says:

== Compilation error in file lib/example_17x_app_web/views/thing_view.ex ==
** (UndefinedFunctionError) function Example17xAppWeb.view/0 is undefined or private
    Example17xAppWeb.view()
    expanding macro: Example17xAppWeb.__using__/1
    lib/example_17x_app_web/views/thing_view.ex:2: Example17xAppWeb.ThingView (module)
    (elixir 1.15.5) expanding macro: Kernel.use/2
    lib/example_17x_app_web/views/thing_view.ex:2: Example17xAppWeb.ThingView (module)

All there is in each repo is a thing controller (which is a singleton and only has a show action) and a thing controller test.

Suspected Reason

Or am I misunderstanding something here? And thanks again for all your help!

beerlington commented 1 year ago

@theirishpenguin Thanks for the examples. I think the immediate issue is the use of use Example17xAppWeb, :view in your view, without a defined view function in your Example17xAppWeb module. Looking at our app that we've been maintain for a while, we are still using a view function.

theirishpenguin commented 1 year ago

@beerlington Thanks. Yes, I think that newly generated phoenix 1.7.7 apps don't have this view function at all (I think it is deprecated but I'm not super familiar with this area). I'm just trying to figure out what the idiomatic thing to do here now when using JA Serializer in phoenix 1.7.7. I can start looking into the new ways of view rendering that phoenix 1.7.7 offers - but I wasn't sure if that was fundamentally incompatible with JA Serializer.

I am guessing my conservative option is to generate a phoenix 1.6.X app and upgrade it to phoenix 1.7.7 (as Jose seems to have suggested in the workaround in my original description).

But, then we'll all still have a problem when we try to create new phoenix apps with phoenix 1.7.7 and future versions of phoenix. And I'm interested to know if there are any JA serializer plans to work to improve phoenix 1.7.7 integration (or if anything needs to change). Put another way, what needs to change in the JA Serializer README example to work with phoenix 1.7.7?

image

(Is the above example now obsolete in terms of the latest Phoenix release?)

Thanks for all your help. Really appreciate it.

beerlington commented 1 year ago

That's a good question about how to move forward. The docs you linked to in your first comment say this:

These features provide a unified rendering model for applications going forward with a new and improved way to write UIs, but they are a deviation from previous practices. Most large, established applications are probably best served by continuing to depend on Phoenix.View.

It doesn't sound like Phoenix.View is going away any time soon, so we'll likely encourage this approach for the time being. I'm open to suggestions on how to move forward for people opting not to use this approach, but without a production app going in that direction, I can't say it's a priority for me.

theirishpenguin commented 1 year ago

Great to get your above input @beerlington , cheers!

It doesn't sound like Phoenix.View is going away any time soon, so we'll likely encourage this approach for the time being.

Yes, indeed :+1:

I'm open to suggestions on how to move forward for people opting not to use this approach, but without a production app going in that direction, I can't say it's a priority for me.

As an experiment (and with very little knowledge of the new Component/Template approach in the latest Phoenix), I have managed to get my broken phoenix 1.7.7 example working with ja_serializer for a basic scenario https://github.com/theirishpenguin/example_17x_app/pull/3 It is quite hacky, but I wonder if this illustrates that there may be a reasonable path towards using ja_serializer with phoenix 1.7.7 without bringing in the phoenix_view fallback package?

beerlington commented 1 year ago

What you've got in your pull request makes sense to me. As I mentioned in your example app PR, having to override the type will be required because we try to infer the type from the module name by default.

I wonder if there's a way to override the module name that Phoenix is looking for so you don't have to use a name like :"Elixir.Example17xAppWeb.ThingJSON-API". That part does feel a bit hacky. Maybe put_view/2? https://hexdocs.pm/phoenix/1.7.7/Phoenix.Controller.html#put_view/2

theirishpenguin commented 1 year ago

@beerlington That's great advice, thanks. I've just tried it out at https://github.com/theirishpenguin/example_17x_app/pull/4 and it seems to work a treat. Also fixes the inferring of the "type" getting rid of the type function in the view.

Do you see any reason why this kind of approach should not be used in production? (No pressure :smile: )

beerlington commented 1 year ago

Do you see any reason why this kind of approach should not be used in production?

Nope, I think this is a perfectly valid approach. If you have a lot of controllers, it might get a little repetitive. But there are ways to DRY that up so it's easier to maintain.

theirishpenguin commented 1 year ago

Great to hear that @beerlington! I think we can bring the curtains down on this one. I have created a single PR to illustrate how someone can use the above approach to bring ja_serializer into a vanilla phoenix 1.7.X project - https://github.com/theirishpenguin/example_17x_app/pull/5 - just in case it might be of use to someone else reading this in future.

Thanks again for all the help. Very impressed with how quickly you replied and I've learned at lot from your advice :raised_hands: