Supress IgniterJs warnings when not installed#1779
Supress IgniterJs warnings when not installed#1779kepi wants to merge 1 commit intonaymspace:developfrom
Conversation
As IgniterJs is optional dependency, we should send warnigns to output when it is missing.
| # IgniterJs is an optional dependency - suppress warnings when not installed | ||
| @compile {:no_warn_undefined, {IgniterJs.Helpers, :read_and_validate_file, 1}} | ||
| @compile {:no_warn_undefined, {IgniterJs.Parsers.Javascript.Parser, :insert_imports, 3}} | ||
| @compile {:no_warn_undefined, {IgniterJs.Parsers.Javascript.Parser, :extend_hook_object, 3}} |
There was a problem hiding this comment.
Rather than suppressing the warning, we should fix the root cause.
Although we already check whether the Igniter is installed using Code.ensure_loaded?(Igniter) (see https://github.com/naymspace/backpex/blob/0.17.1/lib/mix/tasks/backpex.install.ex#L48), the intall task depends on both the igniter and igniter_js. If you have installed igniter, but not igniter_js, the installation command is compiled and a warning is logged because igniter_js is missing.
I suggest that we only load the install MIx task if both igniter and igniter_js are installed.
There was a problem hiding this comment.
Definitely agree. If I don't understand it wrong, II suppose you can close this PR as it is no longer needed. Thanks.
Edit: sorry, just realized I can close it myself...
There was a problem hiding this comment.
Yes, I fixed this in #1821.
Anyway, thanks for the contribution. Otherwise, we would not have noticed this issue!
2 participants
Honestly, I'm completely unsure if this is correct way or if this should be merged. My only motivation is that I can't work on backpex without it as every file I edit in my project gets "==> backpex" at the top from
mix format🥹I kind of thing that this might be bug in Mix, but as complete Elixir noob, I might be completely wrong.
Anyway, as IgniterJs is optional dependency I believe we shouldn't send warnings warnigns to output when it is missing.
Feel free to close this without merging, I'l simply leave it locally or install IgniterJs in my project. I just thought it might be helpful to somebody else.