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

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

Open
theirishpenguin opened this issue Aug 29, 2023 · 12 comments

Comments

@theirishpenguin
Copy link

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

@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
Copy link
Author

theirishpenguin commented Aug 29, 2023

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

  • example_16x_app Phoenix 1.6.16 example showing integration with ja serializer working ✔️
  • example_17x_app Phoenix 1.7.7 example showing my problem integrating ja serializer ❌

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

  • A newly generated Phoenix 1.7.7 app no longer has a classic "view". Hence, view is not a concept that exists in Phoenix 1.7.7...
  • ... unless - perhaps in your case - you have upgraded an app from 1.6.X to 1.7.X (I may be wrong of course!). This does some backwards compatible magic - as noted in my original issue description.

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

@beerlington
Copy link
Contributor

@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
Copy link
Author

theirishpenguin commented Aug 29, 2023

@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
Copy link
Contributor

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
Copy link
Author

theirishpenguin commented Aug 30, 2023

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 👍

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 theirishpenguin/example_17x_app#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
Copy link
Contributor

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
Copy link
Author

theirishpenguin commented Aug 31, 2023

@beerlington That's great advice, thanks. I've just tried it out at theirishpenguin/example_17x_app#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 😄 )

@beerlington
Copy link
Contributor

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
Copy link
Author

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 - theirishpenguin/example_17x_app#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 🙌

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

2 participants