mojotech / torch

A rapid admin generator for Elixir & Phoenix
Other
1.12k stars 80 forks source link

Dependency on phoenix_ecto #475

Open axelson opened 3 months ago

axelson commented 3 months ago

In the Readme and mix.exs there's no mention of phoenix_ecto but trying to edit a resource without phoenix_ecto loaded results in an exception like:

    ** (Protocol.UndefinedError) protocol Phoenix.HTML.FormData not implemented for #Ecto.Changeset<action: nil, changes: %{}, errors: [], data: #Pomodoro.Schemas.PomodoroLog<>, valid?: true, ...> of type Ecto.Changeset (a struct). This protocol is implemented for the following type(s): Atom, Map, Plug.Conn
        (phoenix_html 4.1.1) lib/phoenix_html/form_data.ex:1: Phoenix.HTML.FormData.impl_for!/1

Adding a dep like {:phoenix_ecto, "~> 4.6"} to your projects mix.exs fixes this error.

To resolve this I think either ensuring :phoenix_ecto is installed to the Readme, or directly depend on :phoenix_ecto in Torch's mix.exs.

cpjolicoeur commented 3 months ago

This is the first time a report like this has come in. Could you provide a minimal mix.exs file that demonstrates this issue?

I'm inferring that you are running a Phoenix project, and using Ecto, but decided to manually not use phoenix_ecto on that project as well?

axelson commented 3 months ago

Sure! Here you go: https://github.com/axelson/torch_repro_phoenix_ecto

Yeah it isn't a common setup. In my actual project I generated an ecto-only project, then sometime later I decided I wanted a web frontend for it so I generated a separate phoenix project for it, but at that time it was only for visualization so I didn't realize that I hadn't added :phoenix_ecto.

cpjolicoeur commented 3 months ago

OK, let me look at this and consider it. Torch was (is currently) only intended to be used on a Phoenix project as it depends quite heavily (as you found out) on Phoenix and its feature libraries. I'm not sure we want to fully decouple that. We may end up just adding a note to the README docs about the issue you found.

axelson commented 2 months ago

Yeah I'm definitely not suggesting to decouple Torch from Phoenix. Just mentioning a part of Phoenix that Torch depends on implicitly (and hoping to make it slightly more explicit).