Skip to content

Upgrade :esbuild to minimum version when upgrading to 1.1 #4011

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 6, 2025
Merged

Upgrade :esbuild to minimum version when upgrading to 1.1 #4011

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0b58af4
Upgrade :esbuild to minimum version when upgrading to 1.1
aifrak Oct 5, 2025
e50a9ab
Pass quoted code to set_dep_option/4
aifrak Oct 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions lib/phoenix_live_view/igniter/upgrade_to_1_1.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,12 @@ if Code.ensure_loaded?(Igniter) do
config_exs_vsn = Rewrite.Source.version(igniter.rewrite.sources["config/config.exs"])

igniter =
Igniter.Project.Config.configure(
igniter,
igniter
|> Igniter.Project.Deps.add_dep(
{:esbuild, "~> 0.10", runtime: quote(do: Mix.env() == :dev)},
Copy link
Contributor Author

@aifrak aifrak Oct 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, there is a small issue.

Usually, in a freshly generated Phoenix project, we have a line for :esbuild like below in mix.exs for the dependencies:

{:esbuild, "~> 0.8", runtime: Mix.env() == :dev},

Why quote?

quote is used so that it generates runtime: Mix.env() == :dev. Without it, it would generate something like runtime: false .

Consequence of using quote

At the upgrade confirmation, it prints an AST instead of Mix.env() == :dev:

Desired: `{:esbuild, "~> 0.10", [runtime: {:==, [context: Phoenix.LiveView.Igniter.UpgradeTo1_1, imports: [{2, Kernel}]], [{{:., [], [{:__aliases__, [alias: false], [:Mix]}, :env]}, [], []}, :dev]}]}`
Found: `{:esbuild, "~> 0.8", [runtime: true]}`

This might not be so clear for the user...

Alternative to quote

An alternative would be to just add a raw value runtime: :dev, it would look like this:

|> Igniter.Project.Deps.add_dep(
  {:esbuild, "~> 0.10", runtime: :dev},
)

But this would then upgrade AND update the value for :runtime, which might not be ideal, because we just need to upgrade the version without touching the test.

I would need some guidance on this one, because both has some drawbacks. I have chosen the one that has an unclear message, but without modifying something that we should not.

Copy link
Contributor Author

@aifrak aifrak Oct 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Ash team me proposed two alternatives to solve this solutions:

  1. Pass :yes? to add_dep, so that "Desired" and "Found" are not shown and the upgrade is directly done. (discord answer)
  2. Move the quoted value into set_dep_option/4 (discord answer)

I have chosen solution 2. so that it is clearer what the changes are going to happen.

Here the printed result when solution 2. is used:

Desired: `{:esbuild, "~> 0.10"}`
Found: `{:esbuild, "~> 0.8", [runtime: true]}`

The AST is not printed anymore and the versions are shown correctly. A small drawback is that it is not exactly like what is written in mix.exs, because runtime: Mix.env() == :dev is missing.

If this solution is not fine for you, I will make changes for solution 1.

Thanks for the Ash team for their support.

Related issue on Igniter: ash-project/igniter#338

on_exists: :overwrite
)
|> Igniter.Project.Config.configure(
"config.exs",
:esbuild,
app_name,
Expand Down
22 changes: 21 additions & 1 deletion test/phoenix_live_view/igniter/upgrade_to_1_1_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,9 @@ defmodule Phoenix.LiveView.Igniter.UpgradeTo1_1Test do
)
# yes to esbuild
|> run_upgrade(input: "y\n")
|> assert_has_patch("mix.exs", """
+ | {:esbuild, "~> 0.10", runtime: Mix.env() == :dev},
""")
|> assert_has_patch("config/config.exs", """
- | args: ~w(js/app.js --bundle --outdir=../priv/static/assets),
+ | args: ~w(js/app.js --bundle --outdir=../priv/static/assets --alias:@=.),
Expand Down Expand Up @@ -308,6 +311,9 @@ defmodule Phoenix.LiveView.Igniter.UpgradeTo1_1Test do
)
# yes to esbuild (no deps prompt since no existing deps)
|> run_upgrade(input: "y\n")
|> assert_has_patch("mix.exs", """
+ | {:esbuild, "~> 0.10", runtime: Mix.env() == :dev},
""")
|> assert_has_patch("config/config.exs", """
- | args: ["js/app.js", "--bundle", "--outdir=../priv/static/assets"],
+ | args: ["js/app.js", "--bundle", "--outdir=../priv/static/assets", "--alias:@=."],
Expand All @@ -333,6 +339,9 @@ defmodule Phoenix.LiveView.Igniter.UpgradeTo1_1Test do
)
# yes to esbuild
|> run_upgrade(input: "y\n")
|> assert_has_patch("mix.exs", """
+ | {:esbuild, "~> 0.10", runtime: Mix.env() == :dev},
""")
|> assert_has_patch("config/config.exs", """
- | args: ~w(js/app.js --bundle --outdir=../priv/static/assets),
+ | args: ~w(js/app.js --bundle --outdir=../priv/static/assets --alias:@=.),
Expand Down Expand Up @@ -361,6 +370,9 @@ defmodule Phoenix.LiveView.Igniter.UpgradeTo1_1Test do
)
# yes to esbuild (no deps prompt since no existing deps)
|> run_upgrade(input: "y\n")
|> assert_has_patch("mix.exs", """
+ | {:esbuild, "~> 0.10", runtime: Mix.env() == :dev},
""")
|> assert_has_warning(&(&1 =~ "Failed to update esbuild configuration for colocated hooks"))
|> refute_has_notice()
end
Expand All @@ -381,6 +393,9 @@ defmodule Phoenix.LiveView.Igniter.UpgradeTo1_1Test do
)
# no to deps prompt, yes to esbuild
|> run_upgrade(input: "y\n")
|> assert_has_patch("mix.exs", """
+ | {:esbuild, "~> 0.10", runtime: Mix.env() == :dev},
""")
|> assert_has_warning(&(&1 =~ "Failed to update esbuild configuration for colocated hooks"))
|> refute_has_notice()
end
Expand Down Expand Up @@ -444,6 +459,10 @@ defmodule Phoenix.LiveView.Igniter.UpgradeTo1_1Test do
|> assert_has_patch("mix.exs", """
+ | {:lazy_html, ">= 0.0.0", only: :test},
""")
|> assert_has_patch("mix.exs", """
- | {:esbuild, "~> 0.8", runtime: Mix.env() == :dev},
+ | {:esbuild, "~> 0.10", runtime: Mix.env() == :dev}
""")
|> assert_has_patch("config/dev.exs", """
- | reloadable_compilers: [:elixir, :app]
+ | reloadable_compilers: [:phoenix_live_view, :elixir, :app]
Expand Down Expand Up @@ -483,7 +502,8 @@ defmodule Phoenix.LiveView.Igniter.UpgradeTo1_1Test do
defp deps do
[
{:phoenix, "~> 1.7.0"},
{:phoenix_live_view, "~> 0.20.0"}
{:phoenix_live_view, "~> 0.20.0"},
{:esbuild, "~> 0.8", runtime: Mix.env() == :dev},
]
end
end
Expand Down
Loading