burrito-elixir / burrito

Wrap your application in a BEAM Burrito!
MIT License
979 stars 34 forks source link

Upgrade typed_struct to TypedStruct? #157

Open c-alpha opened 2 months ago

c-alpha commented 2 months ago

I tried to build my app with burrito, and ran into the following:

▶ mix deps.compile
==> typed_struct
Compiling 2 files (.ex)
Generated typed_struct app
    warning: redefining module TypedStruct.MixProject (current version defined in memory)
    │
  1 │ defmodule TypedStruct.MixProject do
    │ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    │
    └─ /Users/...redacted.../deps/typedstruct/mix.exs:1: TypedStruct.MixProject (module)

I am using the new(er) TypedStruct which is a drop-in replacement for the "classic" typed_struct, and am hence seeing this warning. Quoting from TypedStruct's README:

NOTE: This is an active fork of the original typedstruct work by Jean-Philippe Cugnet, which seems to be no longer maintained. This version adds type information to Erlang records and makes the project compile under OTP-26 and later.

I have been using the new version with code of mine developed against the old typed_struct without any problems. I hence thought I'd notify you of the new version, and at the same time suggest that instead of:

{:typed_struct, "~> 0.2.0 or ~> 0.3.0", runtime: false},

you could opt to instead use:

{:typedstruct, "~> 0.5", runtime: false},

Hoping to have helped, and looking forward to your thoughts.

doawoo commented 1 month ago

I've seen a few formatter issues that people have brought up with the fork, that still haven't been addressed in that repo. I'd ideally like to allow folks to select which library they'd want.

Mix doesn't support "features" like cargo does, so...

I've hacked up a small "conditional" dep trick in the mix.exs on a local branch that looks something like this:

  @use_new_typed_struct System.get_env("BURRITO_USE_NEW_TYPED_STRUCT") != nil 

  # ...

  defp deps do
    [
      get_typed_struct(),
      {:req, ">= 0.4.0"},
      {:jason, "~> 1.2"},
      {:ex_doc, ">= 0.0.0", only: :dev}
    ]
  end

  # ...

  defp get_typed_struct() do
    # NOTE: This is here for those who want to use the new `:typedstruct` forked library
    # to do so, set BURRITO_USE_NEW_TYPED_STRUCT=1 in your environment.
    if @use_new_typed_struct do
      {:typedstruct, "~> 0.5", runtime: false}
    else
      {:typed_struct, "~> 0.2.0 or ~> 0.3.0", runtime: false}
    end
  end

So setting the env variable BURRITO_USE_NEW_TYPED_STRUCT=1 would select the forked library.

If this seems satisfactory to you, I'd go forward and merge this in. I don't want to drop the original library since it works fine, but I see the case for supporting the new one.

c-alpha commented 1 month ago

Does that mean that when I do a mix deps.compile with the new environmnet variable set, and your change released on Hex.pm, that would do the trick?

doawoo commented 1 month ago

It should! I did some tests locally and it appears to function as intended