elixir-tools / spitfire

Error tolerant parser for Elixir
https://www.elixir-tools.dev
MIT License
75 stars 7 forks source link

requires elixir 1.17? #47

Closed jlgeering closed 4 months ago

jlgeering commented 4 months ago

Macro.Env.define_alias/4 is only available since 1.17, see https://hexdocs.pm/elixir/1.17.0/Macro.Env.html#define_alias/4

there is also Macro.Env.define_import/4 Macro.Env.define_require/4 and their respective expand... variants, all in lib/spitfire/env.ex

(while the mix.exs file says: elixir: "~> 1.15")

jlgeering commented 4 months ago

NB this makes it a little annoying for our project using Ash, as we are still on Elixir 1.16

mhanberg commented 4 months ago

The Spitfire.Env module requires 1.17. The parser itself (the Spitfire module) does not.

The library itself is still in alpha state, so I haven't feature flagged the parts that require more recent Elixir versions yet.

What is your use case?

jlgeering commented 4 months ago

I'm just a regular Ash user and when I saw the new warning after upgrading, it got me worried and I reverted the upgrade, without digging further. It might well be that I can ignore this warning; I just checked: all our tests still pass.

Maybe I just worried for nothing? Based on the tone of your response, it sounds like normal ash users would be fine and this warning should be ignored / will go away once you feature flag certain parts of the codebase?

jlgeering commented 4 months ago

there is still this small worry when you say that spitfire is in alpha state and we are running ash in production.

ash depends on spark which depends on igniter which depends on spitfire soooooo this is probably more a question for the ash-project people (@zachdaniel ?) and less for you. (The question being: am I safe upgrading to latest ash version ?)

I suspect the answer to be yes, as igniter is described as a code generation library.

jlgeering commented 4 months ago

closing this ticket, as this whole thing is probably "not your problem"

zachdaniel commented 4 months ago

Yes, nothing to worry about :) Ash doesn't depend on it for anything that doesn't happen in a mix task. It's never used in the context of a running application. (in fact it literally can't be). I will also adjust our code to avoid that warning.

zachdaniel commented 4 months ago

Actually, I can't do that, since this one is in spitfire. But these warnings should show up on the dependency itself, right? Our original plan was to require that end users add {:igniter, ..., only: [:dev]} to their application in order to use any of the igniter generators, but that has some cumbersome UX issues. Specifically, we have to define versions of every single mix task that tells the user to add igniter to their project and call the mix task again. So we opted for the simpler approach of just including igniter as a standard dependency, since igniter, spitfire, rewrite, sourceror (etc.) are ultimately not a significant amount of code.

Perhaps we should walk that back though so this code is only ever compiled in dev environments 🤔

jlgeering commented 4 months ago

correct, the warning is shown while compiling spitfire