Make igniter an `only: [:dev, :test]` dependency by default

[zachdaniel]

We originally decided not to go this route because of the additional friction imposed during installation. However, project generators like mix igniter.new and mix phx.new can add :igniter automatically as a dependency.

Additionally a reason I didn't want to do this was because the process of installing packages that don't have an optional dependency on igniter would then "break", but we have resolved this issue via automatically detecting and patching dependencies with conflicting only options.

With this in place, I think we should do the following:

1. adjust igniter.new to do a only: [:dev, :test] on the added igniter dependency.
2. change all packages that use igniter to make it an optional: true dependency.

This prevents any igniter code from shipping to production environments (which isn't really dangerous to do, just annoying), and will make people in general happier with including igniter as a dependency of their projects.

@zachallaun thoughts?

[zachdaniel]

This is actually a very easy change to make in igniter itself, we just change the generated dep to optional. The only place it is a bit more complicated is on the packages that use igniter which have to communicate to users that they must first install igniter. When generating tasks, we would change the --optional to be the default, and use --no-optional to make it required.

[zachallaun]

I think I'm for this, but want to make sure I understand the impact on library authors and end users.

For library authors:

1. Igniter dependency changes to {:igniter, "...", only: [:dev, :test], optional: true}
2. Igniter tasks need to be converted to handle the case where Igniter isn't present

For end users:

1. Igniter must now be specified as a dev/test dependency in order to use tasks that require it: {:igniter, "...", only: [:dev, :test]}

---

Something we could do to help document this is introduce a Igniter.Mix.Task.optional_dependency_docs() that returns an admonition block that library authors can easily stick in their task moduledoc. It could be something like this:

def optional_dependency_docs do
  """
  > #### This task requires Igniter {: .warning}
  >
  > To use this task, you must add `:igniter` to your deps in `mix.exs`:
  >
  > ```elixir
  > {:igniter, "~> #{current_igniter_version()}", only: [:dev, :test]}
  > ```
  """
end

# and then can be used:
defmodule Mix.Tasks.MyIgniterTask do
  @moduledoc """
  #{@shortdoc}

  #{Igniter.Mix.Task.optional_dependency_docs()}

  ...
  """
  ...
end

[zachdaniel]

We don't need only: [:dev, :test] for #1, just optional: true, but otherwise yes your understanding is correct.

Agreed on the clarity changes you describe there. With that said, mix igniter.gen.task should do that automatically so that it shows in the docs etc. We can also probably write an upgrader for this.

[zachallaun]

We don't need only: [:dev, :test] for #1, just optional: true, but otherwise yes your understanding is correct.

Gotcha -- optional: true means the dep isn't included in any env unless the end user includes it, in which case it'll be in whatever env they put it in (like [:dev, :test]).

With that said, mix igniter.gen.task should do that automatically so that it shows in the docs etc.

Yes, I'm thinking that the call would be included in the generated moduledoc unless --no-optional is specified.

Another thing we could do is have mix igniter.gen.task default to optional or not based on how Igniter is specified in the library's deps. If it's optional, the default is --optional. If it's not optional, the default is --no-optional. IMO, that'll be a better UX for the vast majority of cases. Otherwise, at least some people will be running the generator and think, "shit, I meant for that to be non-optional" (or whatever the non-default is).

We can also probably write an upgrader for this.

I was thinking about that a bit. Since it's up to the library author to decide whether Igniter is optional, the upgrade path is non-linear. Some will want to switch to optional and then have their tasks automatically converted, while others will want to make Igniter required. Perhaps this belongs in mix igniter.refactor.optional_task [my_lib.my_task] (or something)?

[zachdaniel]

Another thing we could do is have mix igniter.gen.task default to optional or not based on how Igniter is specified in the library's deps. If it's optional, the default is --optional.

Oh, thats clever. 👍

[zachdaniel]

I was thinking about that a bit. Since it's up to the library author to decide whether Igniter is optional, the upgrade path is non-linear. Some will want to switch to optional and then have their tasks automatically converted, while others will want to make Igniter required. Perhaps this belongs in mix igniter.refactor.optional_task [my_lib.my_task] (or something)?

Its also a pretty easy change to make manually, maybe we just let it be manual :D

[zachallaun]

Its also a pretty easy change to make manually, maybe we just let it be manual :D

I'm also totally fine with this. We should update the Igniter.Mix.Task moduledocs to give more examples of the basic "shape" of both optional and non-optional Igniter tasks.

[kantum]

My app was failing with errors about another library.

 2024-11-05T16:58:09.228 app[d8901e2c623528] cdg [info] WARN Reaped child process with pid: 381 and signal: SIGUSR1, core dumped? false
 2024-11-05T16:58:21.189 app[d8901e2c623528] cdg [info] 16:58:21.186 request_id=GAUg_KBGwDy3ofAAAAYB [info] GET /
 2024-11-05T16:58:21.192 app[d8901e2c623528] cdg [info] load JSON for humbleicons icon family: "/app/deps/iconify_ex/assets/node_modules/@iconify/json/json/humbleicons.json"
 2024-11-05T16:58:21.196 app[d8901e2c623528] cdg [info] Logger - error: {removed_failing_handler,disco_log_handler}
 2024-11-05T16:58:21.197 app[d8901e2c623528] cdg [info] 16:58:21.193 request_id=GAUg_KBGwDy3ofAAAAYB [error] Iconify error with icon: "bars": No icon set found at `/app/deps/iconify_ex/assets/node_modules/@iconify/json/json/humbleicons.json` for the icon `bars`. Find icon sets at https://icones.js.org
 2024-11-05T16:58:21.197 app[d8901e2c623528] cdg [info] Iconify.icon_error/2 @ (iconify_ex 0.5.3) lib/iconify.ex:765
 2024-11-05T16:58:21.197 app[d8901e2c623528] cdg [info] Iconify.list_json_svgs/3 @ (iconify_ex 0.5.3) lib/iconify.ex:690
 2024-11-05T16:58:21.197 app[d8901e2c623528] cdg [info] Iconify.get_svg/3 @ (iconify_ex 0.5.3) lib/iconify.ex:653
 2024-11-05T16:58:21.197 app[d8901e2c623528] cdg [info] Iconify.svg_as_is/3 @ (iconify_ex 0.5.3) lib/iconify.ex:592
 2024-11-05T16:58:21.197 app[d8901e2c623528] cdg [info] Iconify.do_prepare_icon_css/4 @ (iconify_ex 0.5.3) lib/iconify.ex:553
 2024-11-05T16:58:21.197 app[d8901e2c623528] cdg [info] Iconify.prepare_icon_css/2 @ (iconify_ex 0.5.3) lib/iconify.ex:536
 2024-11-05T16:58:21.197 app[d8901e2c623528] cdg [info] Iconify.prepare/2 @ (iconify_ex 0.5.3) lib/iconify.ex:94
 2024-11-05T16:58:21.197 app[d8901e2c623528] cdg [info] Iconify.iconify/1 @ (iconify_ex 0.5.3) lib/iconify.ex:23
 2024-11-05T16:58:21.197 app[d8901e2c623528] cdg [info] load JSON for heroicons-solid icon family: "/app/deps/iconify_ex/assets/node_modules/@iconify/json/json/heroicons-solid.json"
 2024-11-05T16:58:21.541 app[d8901e2c623528] cdg [info] 16:58:21.540 request_id=GAUg_KBGwDy3ofAAAAYB [info] Sent 500 in 354ms
 2024-11-05T16:58:21.543 app[d8901e2c623528] cdg [info] 16:58:21.541 request_id=GAUg_KBGwDy3ofAAAAYB [error] ** (RuntimeError) No icon set found at `/app/deps/iconify_ex/assets/node_modules/@iconify/json/json/heroicons-solid.json` for the icon `question-mark-circle`. Find icon sets at https://icones.js.org

Turns out it was igniter that was causing the issue, adding only: :dev solved the problem

[zachdaniel]

I do not see any possible way that igniter could cause that error. I think it's more likely that recompiling does fixed your problem TBH

[kantum]

That's why it took me some time before I add this option :)

The failure is happening once I run in production in fly.io, so after getting through docker building process...

I just retried and got the same result.

From a working state:

diff --git a/mix.exs b/mix.exs
index d02e9dc..eb626ca 100644
--- a/mix.exs
+++ b/mix.exs
@@ -69,7 +69,7 @@ defmodule Durot.MixProject do
       {:ex_cldr_plugs, "~> 1.3"},
       {:ex_cldr_numbers, "~> 2.33"},
       {:ex_cldr_dates_times, "~> 2.20"},
-      {:igniter, "~> 0.4.0", only: :dev}
+      {:igniter, "~> 0.4.0"}
     ]
   end

500 error in fly.io

2024-11-06T00:14:51Z app[d8901e2c623528] cdg [info]00:14:51.384 request_id=GAU4zoG23C4xhdMAAAYR [info] GET /
2024-11-06T00:14:51Z app[d8901e2c623528] cdg [info]load JSON for humbleicons icon family: "/app/deps/iconify_ex/assets/node_modules/@iconify/json/json/humbleicons.json"
2024-11-06T00:14:51Z app[d8901e2c623528] cdg [info]Logger - error: {removed_failing_handler,disco_log_handler}
2024-11-06T00:14:51Z app[d8901e2c623528] cdg [info]00:14:51.391 request_id=GAU4zoG23C4xhdMAAAYR [error] Iconify error with icon: "bars": No icon set found at `/app/deps/iconify_ex/assets/node_modules/@iconify/json/json/humbleicons.json` for the icon `bars`. Find icon sets at https://icones.js.org
2024-11-06T00:14:51Z app[d8901e2c623528] cdg [info]    Iconify.icon_error/2 @ (iconify_ex 0.5.5) lib/iconify.ex:753
2024-11-06T00:14:51Z app[d8901e2c623528] cdg [info]    Iconify.list_json_svgs/3 @ (iconify_ex 0.5.5) lib/iconify.ex:678
2024-11-06T00:14:51Z app[d8901e2c623528] cdg [info]    Iconify.get_svg/3 @ (iconify_ex 0.5.5) lib/iconify.ex:641
2024-11-06T00:14:51Z app[d8901e2c623528] cdg [info]    Iconify.svg_as_is/3 @ (iconify_ex 0.5.5) lib/iconify.ex:580
2024-11-06T00:14:51Z app[d8901e2c623528] cdg [info]    Iconify.do_prepare_icon_css/4 @ (iconify_ex 0.5.5) lib/iconify.ex:541
2024-11-06T00:14:51Z app[d8901e2c623528] cdg [info]    Iconify.prepare_icon_css/2 @ (iconify_ex 0.5.5) lib/iconify.ex:524
2024-11-06T00:14:51Z app[d8901e2c623528] cdg [info]    Iconify.prepare/2 @ (iconify_ex 0.5.5) lib/iconify.ex:84
2024-11-06T00:14:51Z app[d8901e2c623528] cdg [info]    Iconify.iconify/1 @ (iconify_ex 0.5.5) lib/iconify.ex:22
2024-11-06T00:14:51Z app[d8901e2c623528] cdg [info]load JSON for heroicons-solid icon family: "/app/deps/iconify_ex/assets/node_modules/@iconify/json/json/heroicons-solid.json"

The Iconify (best site ever BTW) library is not really clean, there are a lot of warnings when building it, so maybe the problem comes from there.

I will try to make a reproduction.

[kantum]

Here is a reproduction:

https://github.com/kantum/iconify_igniter

[zachdaniel]

I'll look when I'm back from vacation unless @zachallaun has time. But I can't conceive of any possible way this could be igniters fault 😅 these libraries have nothing to do with each other and igniter doesn't do anything that could impact other deps.

[mayel]

Oh does including igniter/rewrite/sourcerer somehow cause Mix to be available in a release? because iconify_ex does Code.ensure_loaded?(Mix) as a sneaky way to check for :dev environment (since by default all deps are in :prod)

[zachallaun]

Oh does including igniter/rewrite/sourcerer somehow cause Mix to be available in a release?

Maybe? Igniter tasks wrap Mix tasks, so presumably there's some dependency reference from Igniter back to Mix. I sort of thought that Mix was always stripped from releases, but maybe not.

@mayel As a test, could you try replacing this runtime check with this compile-time one:

if Mix.env() == :prod do
  def dev_env?, do: false
else
  def dev_env?, do: true
end

[mayel]

Only issue with that is that it'd always be false, since mix deps are compiled with :prod env even in dev environments.

[zachallaun]

Only issue with that is that it'd always be false, since mix deps are compiled with :prod env even in dev environments.

Ah, you're right; never mind.

[mayel]

Ok I'm attempting a workaround (putting env in the config), but still curious why this was happening...