Upgrade :esbuild to minimum version when upgrading to 1.1#4011
Upgrade :esbuild to minimum version when upgrading to 1.1#4011SteffenDE merged 2 commits intophoenixframework:mainfrom
Conversation
| igniter, | ||
| igniter | ||
| |> Igniter.Project.Deps.add_dep( | ||
| {:esbuild, "~> 0.10", runtime: quote(do: Mix.env() == :dev)}, |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
The Ash team me proposed two alternatives to solve this solutions:
- Pass
:yes?toadd_dep, so that "Desired" and "Found" are not shown and the upgrade is directly done. (discord answer) - 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
|
Nice! Thank you :) |
* Upgrade :esbuild to minimum version when upgrading to 1.1 * Pass quoted code to set_dep_option/4
2 participants
This PR is a proposal to upgrade
:esbuildto the minimum version0.10while running Igniter in order to avoid errors on joining environment variables.Please check my main concern about showing an unclear message because of the usage of
quote.