cpursley / walex

Postgres change events (CDC) in Elixir
MIT License
276 stars 14 forks source link

DSL not working #34

Closed DohanKim closed 7 months ago

DohanKim commented 7 months ago

problem: using DSL like README gives a compilation error (v 3.6.0)

reason: wrong macro definition

the codes in the WalEx.Event is defining macro like below.

defmodule MacroModule do
  defmacro __using__(_) do
    quote do
      defmacro define_hello(do: block) do
        quote do
          def hello() do
            unquote(block).("hello world")
          end
        end
      end
    end
  end
end

defmodule Test do
  use MacroModule

  define_hello(do: fn x -> IO.puts(x) end)
end

Test.hello()

this code gives an error.

     error: undefined function define_hello/1 (there is no such import)

       define_hello(do: fn x -> IO.puts(x) end)
        ^

You intended something like the one below per README.

defmodule MacroModule do
  defmacro __using__(_) do
    quote do
      import MacroModule
    end
  end

  defmacro define_hello(do: block) do
    quote do
      def hello() do
        unquote(block).("hello world")
      end
    end
  end
end

defmodule Test do
  use MacroModule

  define_hello(do: fn x -> IO.puts(x) end)
end

Test.hello()
cpursley commented 7 months ago

Super, thank you @DohanKim !

What do you think of putting the Dsl logic into it's own module (dsl.ex or something like that)? This is something I've wanted to do for a while and keep event.ex just for the event struct & casting stuff.

cpursley commented 7 months ago

@DohanKim heads up that I just merged some additional changes to master which may break your PR. I think the main thing will be getting the events pid for tests will now work like this:

events_pid = find_worker_pid(destinations_supervisor_pid, DestinationsEventModules)

I moved Event under destinations and renamed the nodule to EventModules.

DohanKim commented 7 months ago

I agree with putting DSL logic into a separate module. let me try to implement it.

DohanKim commented 7 months ago

https://github.com/cpursley/walex/pull/36/commits/4a9a19a7318f650a4a0d66da562cc0c0b53de7be

I think it's more clear to users than importing the Dsl macro implicitly in WalEx.Event.

DohanKim commented 7 months ago

it seems that you are not using the word and concept of "DSL" anymore in the README file, so making the macro imported implicitly look more natural. changed the PR accordingly