ryanwinchester / uuidv7

UUIDv7 for Elixir (and Ecto)
MIT License
34 stars 3 forks source link

Not able to get started with a clean, non-Ecto use case. #6

Closed octowombat closed 6 months ago

octowombat commented 6 months ago

Hi there,

Firstly, thanks for taking the time to create and open-source this library as I have been looking at writing a UUIDv7 capability into one of my apps for a while and came across this after seeing your fork of uniq. My application doesn't use Ecto at all so I was pleased to see that was optional.

However after adding the dependency I could not get it to compile due to this error;

==> uuid_v7
Compiling 3 files (.ex)
     error: module Ecto.Type is not loaded and could not be found
     │
 184 │     use Ecto.Type
     │     ^^^^^^^^^^^^^
     │
     └─ lib/uuidv7.ex:184: UUIDv7 (module)

== Compilation error in file lib/uuidv7.ex ==
** (CompileError) lib/uuidv7.ex: cannot compile module UUIDv7 (errors have been logged)
    (elixir 1.16.2) expanding macro: Kernel.use/1
    lib/uuidv7.ex:184: UUIDv7 (module)
    (elixir 1.16.2) expanding macro: Kernel.if/2
    lib/uuidv7.ex:183: UUIDv7 (module)
could not compile dependency :uuid_v7, "mix compile" failed. Errors may have been logged above. You can recompile this dependency with "mix deps.compile uuid_v7 --force", update it with "mix deps.update uuid_v7" or clean it with "mix deps.clean uuid_v7"

I decided to remove my application specifically in case of some clash there (although as I say, I don't use Ecto in it and your library correctly states Ecto as optional). To this end I created a brand new Elixir app with mix new test_client --sup and added the dependency only in my mix.exs file as normal and the same error appeared.

I had come across something similar in a previous project I was working on and I recalled reading this article from Oct. 2022 on Elixir v1.14.1. So I tried to locally change your UUIDv7 module accordingly (not worrying about being DRY at all) by following the findings in that article, condensed to;

if Code.ensure_loaded?(Ecto.Type) do
  defmodule UUIDv7 do
    # Put complete code here including the `use Ecto.Type` line without the internal `if` clause
    ...
  end
else
  defmodule UUIDv7 do
    # Put code here excluding the `Ecto.Type` lines
    ...
  end
end

When I referenced this local code with in my mix.exs file (using the :path option) it compiled ok, and I was able to use it perfectly.

Have you encountered this at all? Am I doing something wrong perhaps? Thanks Mike (aka Willy Wombat/@octowombat)

octowombat commented 6 months ago

I have forked the repo to prepare a PR if it turns out to be needed 😉

ryanwinchester commented 6 months ago

Thanks for catching this.

What a strange case, since I've used the Code.ensure_loaded?/1 check outside of modules many times before.

I'm making a PR for this and adding a UUIDv7.Type module.

octowombat commented 6 months ago

Ok great. I solved it using the trick in the link together with some using macros to DRY the code. Your way is more transparent though.

ryanwinchester commented 6 months ago

Ok great. I solved it using the trick in the link together with some using macros to DRY the code. Your way is more transparent though.

Yeah, I'd like to avoid tricks in most cases. It pains me to add the .Type but I think it's better in the long run to just be plain and clear.