Phoenix html 4 compatibility #147

Hermanlangner · 2024-04-26T10:56:25Z

On our App we use exq_ui, and I was going on a dependency update mission.

Exq_ui is using phoenix_html: 3.x but phoenix_html: 4+ has compability issues with it.
Namely in use Phoenix.HTML
As in the error message here

To keep compatibility with previous versions, add {:phoenix_html_helpers, "~> 1.0"} to your mix.exs deps
and then, instead of "use Phoenix.HTML", you might:

    import Phoenix.HTML
    import Phoenix.HTML.Form
    use PhoenixHTMLHelpers

I did exactly as the prompt suggested, which should at least add compatibility for a while. It does generate warnings, but that would involve changing the helpers and I'm not sure what the author would prefer to change it to.

This PR is simply enough to bump dependencies

ananthakumaran · 2024-04-27T03:26:59Z

I am open to 4.x, but we need to support 3.x for some time, I am not sure about the reason for the changes in mix.lock file, did you run mix deps.update?. Anyhow see if you can get rid of the warnings and not make changes to mix.lock file.

Hermanlangner · 2024-04-27T06:31:02Z

I am open to 4.x, but we need to support 3.x for some time, I am not sure about the reason for the changes in mix.lock file, did you run mix deps.update?. Anyhow see if you can get rid of the warnings and not make changes to mix.lock file.

I did not consider backwards compatibility, so I did a deps.update all.
I'll revert that but and remove the warnings.
I will still need to add the package for backwards compability though

This reverts commit daacb64.

Hermanlangner · 2024-04-29T07:24:19Z

config/test.exs

@@ -1,7 +1,7 @@
 import Config

 # Print only warnings and errors during test
-config :logger, level: :warn
+config :logger, level: :warning


This specifically deals with the warning

warning: :logger :level has been set to :warn in config files, please use :warning instead (logger 1.16.1) lib/logger/app.ex:102: Logger.App.default_level/0 (logger 1.16.1) lib/logger/app.ex:35: Logger.App.start/2 (kernel 9.2) application_master.erl:293: :application_master.start_it_old/4

Hermanlangner · 2024-04-29T07:25:24Z

lib/exq_ui_web.ex

-      use Phoenix.HTML
+      import Phoenix.HTML
+      import Phoenix.HTML.Form
+      use PhoenixHTMLHelpers


This is to provide backwards compability with phoenix_html 3.3 through the helpers package
{:phoenix_html_helpers, "~> 1.0"}, as the built in recommendation in upgrading

Hermanlangner · 2024-04-29T07:26:15Z

lib/exq_ui_web/live/busy_live/index.html.heex

+        <td>
+          <.link navigate={Routes.queue_show_path(@socket, job.queue)} class="nounderline">
+            <%= job.queue %>
+          </.link>


Got rid of the warning to that live_redirect is deprecated

Hermanlangner · 2024-04-29T07:27:06Z

lib/exq_ui_web/live/components/nav_bar_component.html.heex

+          <.link navigate={Routes.dashboard_path(@socket)} class={nav_link_class(@socket, "Busy")}>
+            Busy
+          </.link>
        </li>


because I can't render ~H in the helper classes, I split out the nav_link helper to provide the class but we have to use the ./link here

Hermanlangner · 2024-04-29T07:28:53Z

lib/exq_ui_web/live/components/table_component.html.heex

-                  <td class={column[:text_break] && "text-break"}><%= accessor.(item) %></td>
+                <%= for column <- @columns do %>
+                  <td class={column[:text_break] && "text-break"}>
+                    <%= if column[:link] do %>


Similar to the nav_link approach before, to cator for the least amount of changes, I allowed colums to provide an optional field :link to determind whether it is one.
There are other more invasive/workflow changing ones, I felt that this way was the least disruptive. I'm happy to consider the other approaches

Hermanlangner · 2024-04-29T07:31:08Z

mix.exs

@@ -36,9 +36,10 @@ defmodule ExqUI.MixProject do
    [
      {:exq, ">= 0.16.2"},
      {:exq_scheduler, "~> 1.0", optional: true},
-      {:phoenix_live_view, "~> 0.18"},
+      {:phoenix_live_view, "~> 0.20.12"},


This was the last version that supported Elixir 12
phoenixframework/phoenix_live_view@83f3f84

all the mix lock updates were for the live view and html helpers. I tried keeping it to a minimum to make them work

Hermanlangner · 2024-04-29T07:39:13Z

@ananthakumaran
I've tried keeping changes to the minimum, for the html I did use mix format since I couldn't get the indentation looking consistently, there's nothing that changed in format that's not out of the box elixir formatting.
Apologies for the few extra line changes.

For the tests, they were very brittle in expecting changes on the same line, I expanded and red green tested to make sure they were correct, they'll now not be coupled to the html structure as they were before.
In the future it's probably even better to be more explicit than a regex selector.

I've tried pointing out the changes I made to make it easier to review.

I've kept as much to the bare minimum as possible.

I had trouble getting my local back to 12 and 13 so I've not been able to test them, as even the vms aren't great anymore through asdf.

I feel the ci runners should be updated for newer versions of elixir, but this is entirely your call.

Phoenix html 3.3 is fully supported thanks to the helpers package,
but I don't know about older versions of elixir. I'm happy to make small adjustments to cater for it.

I do feel that package versioning exists for this reason of supporting the newer and dropping the older, however this is ultimately your package, and you decide these things.

If we can't reach some point of agreement I'll be forced to fork it, as my time is limited, but I'm doing this to try and give back to a library that's been incredibly useful for us.

ananthakumaran · 2024-04-30T04:19:14Z

mix.lock

-  "phoenix": {:hex, :phoenix, "1.7.7", "4cc501d4d823015007ba3cdd9c41ecaaf2ffb619d6fb283199fa8ddba89191e0", [:mix], [{:castore, ">= 0.0.0", [hex: :castore, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:phoenix_pubsub, "~> 2.1", [hex: :phoenix_pubsub, repo: "hexpm", optional: false]}, {:phoenix_template, "~> 1.0", [hex: :phoenix_template, repo: "hexpm", optional: false]}, {:phoenix_view, "~> 2.0", [hex: :phoenix_view, repo: "hexpm", optional: true]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 2.6", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:plug_crypto, "~> 1.2", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}, {:websock_adapter, "~> 0.5.3", [hex: :websock_adapter, repo: "hexpm", optional: false]}], "hexpm", "8966e15c395e5e37591b6ed0bd2ae7f48e961f0f60ac4c733f9566b519453085"},
-  "phoenix_html": {:hex, :phoenix_html, "3.3.2", "d6ce982c6d8247d2fc0defe625255c721fb8d5f1942c5ac051f6177bffa5973f", [:mix], [{:plug, "~> 1.5", [hex: :plug, repo: "hexpm", optional: true]}], "hexpm", "44adaf8e667c1c20fb9d284b6b0fa8dc7946ce29e81ce621860aa7e96de9a11d"},
+  "phoenix": {:hex, :phoenix, "1.7.12", "1cc589e0eab99f593a8aa38ec45f15d25297dd6187ee801c8de8947090b5a9d3", [:mix], [{:castore, ">= 0.0.0", [hex: :castore, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:phoenix_pubsub, "~> 2.1", [hex: :phoenix_pubsub, repo: "hexpm", optional: false]}, {:phoenix_template, "~> 1.0", [hex: :phoenix_template, repo: "hexpm", optional: false]}, {:phoenix_view, "~> 2.0", [hex: :phoenix_view, repo: "hexpm", optional: true]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 2.7", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:plug_crypto, "~> 1.2 or ~> 2.0", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}, {:websock_adapter, "~> 0.5.3", [hex: :websock_adapter, repo: "hexpm", optional: false]}], "hexpm", "d646192fbade9f485b01bc9920c139bfdd19d0f8df3d73fd8eaf2dfbe0d2837c"},
+  "phoenix_html": {:hex, :phoenix_html, "4.1.1", "4c064fd3873d12ebb1388425a8f2a19348cef56e7289e1998e2d2fa758aa982e", [:mix], [], "hexpm", "f2f2df5a72bc9a2f510b21497fd7d2b86d932ec0598f0210fed4114adc546c6f"},


would it be possible to keep phoenix_html 3.x here? Because if we use 4.x here, then it would be very hard to tell whether exq_ui supports 3.x or not and easy for future changes to break the support as well.

I will give this a try this evening, as well as check if it's compatible with an app that's running on 3.x

ananthakumaran · 2024-04-30T04:32:08Z

I had trouble getting my local back to 12 and 13 so I've not been able to test them, as even the vms aren't great anymore through asdf.

I feel the ci runners should be updated for newer versions of elixir, but this is entirely your call.

agree, I am ok with dropping 1.12 (which seems to be missing support for Keyword.validate!), adding newer versions can be handled as a separate PR

I do feel that package versioning exists for this reason of supporting the newer and dropping the older, however this is ultimately your package, and you decide these things.

Compared to other libraries I maintain, exq_ui has to be constantly updated thanks to phoenix and phoenix live view changes :). I try to support last 2 versions. phoenix_html 4.x is brand new (December 2023) and I can't drop support for 3.x as we have projects using exq_ui and depend on 3.x

Phoenix html 3.3 is fully supported thanks to the helpers package,

if this is true, I don't have much issues. The changes looks good so far, the only request I have is keep running CI with phoenix HTML 3.x (which got switch to 4.x)

If we can't reach some point of agreement I'll be forced to fork it, as my time is limited, but I'm doing this to try and give back to a library that's been incredibly useful for us.

Fully understand, don't worry too much. If you run short on time feel free to use a fork for some time. Eventually, I or someone else will get the required changes done.

Hermanlangner · 2024-04-30T14:19:32Z

@ananthakumaran I tried bumping down the dependencies, unfortunately it looks like phoenix_html_helpers provides backwards compatibility to phoenix_html 3.3 but it depends on phoenix_html 4.
It will unfortunately not be possible to update while having 3.x as a dependency, which is a bummer, I was hoping to get best of both worlds.

I fully understand the supporting 2+ versions, and hopefully this work does become useful when it becomes time to make the switch!

Hermanlangner · 2024-04-30T15:20:58Z

If this is picked up in the future, I had still missed some manual testing before I was going to merge, I wrongly copy pasted the routes in the navcomponent. Don't let that slip through

ananthakumaran · 2024-04-30T15:47:28Z

Thanks for the PR. I will come back to this in a month or two and would surely be useful.

ananthakumaran · 2024-06-13T01:31:29Z

@Hermanlangner would you be able to remove the package name change related commits. I want to get this merged. Let me know if you are busy, I can cherry-pick other commits and create a new PR

ananthakumaran · 2024-06-27T05:50:36Z

Master supports phoenix html 4

compatible with phoenix_html 4

691fa40

Hermanlangner added 11 commits April 27, 2024 08:45

reset lock

4daca9d

Minimum lock changes

dbeed76

some warnings gone

e9fe5fd

base component change

daacb64

Revert "base component change"

1550207

This reverts commit daacb64.

more flexible tests

1f10267

column component adjusted

85bf399

delete test more flexible

3ffa8b8

Most warnings gone

1f0acdf

index test more flexible

821401e

standard mix format and remove warnings

4db77fb

Hermanlangner commented Apr 29, 2024

View reviewed changes

ananthakumaran reviewed Apr 30, 2024

View reviewed changes

Hermanlangner closed this Apr 30, 2024

Hermanlangner added 4 commits April 30, 2024 16:23

Update README.md

98a6c34

bump all packages

036b33f

rename for package

1fdf887

silly bug fix

2938e5b

ananthakumaran reopened this Jun 13, 2024

ananthakumaran mentioned this pull request Jun 13, 2024

Fix for the Issue Phoenix.HTML is no longer supported in v4.0 #148

Closed

ananthakumaran closed this Jun 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Phoenix html 4 compatibility #147

Phoenix html 4 compatibility #147

Hermanlangner commented Apr 26, 2024

ananthakumaran commented Apr 27, 2024

Hermanlangner commented Apr 27, 2024

Hermanlangner Apr 29, 2024

Hermanlangner Apr 29, 2024

Hermanlangner Apr 29, 2024

Hermanlangner Apr 29, 2024

Hermanlangner Apr 29, 2024

Hermanlangner Apr 29, 2024

Hermanlangner commented Apr 29, 2024

ananthakumaran Apr 30, 2024

Hermanlangner Apr 30, 2024

ananthakumaran commented Apr 30, 2024

Hermanlangner commented Apr 30, 2024

Hermanlangner commented Apr 30, 2024

ananthakumaran commented Apr 30, 2024

ananthakumaran commented Jun 13, 2024

ananthakumaran commented Jun 27, 2024

Phoenix html 4 compatibility #147

Phoenix html 4 compatibility #147

Conversation

Hermanlangner commented Apr 26, 2024

ananthakumaran commented Apr 27, 2024

Hermanlangner commented Apr 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hermanlangner commented Apr 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ananthakumaran commented Apr 30, 2024

Hermanlangner commented Apr 30, 2024

Hermanlangner commented Apr 30, 2024

ananthakumaran commented Apr 30, 2024

ananthakumaran commented Jun 13, 2024

ananthakumaran commented Jun 27, 2024