Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential enhancements? #2

Open
ngscheurich opened this issue Aug 11, 2023 · 6 comments
Open

Potential enhancements? #2

ngscheurich opened this issue Aug 11, 2023 · 6 comments

Comments

@ngscheurich
Copy link

ngscheurich commented Aug 11, 2023

Hi! My team is using a Channel module paradigm that lends itself nicely to ChannelHandler's capabilities, so I was excited to come across this project. In addition to dispatching handle_eventss, we also dispatch joins and handle_infos, which ends up looking something like this:

def join("lobby" = topic, payload, socket) do
  Lobby.join(topic, payload, socket)
end

def join("room:" <> _id  = topic, payload, socket) do
  Dashboard.join(topic, payload, socket)
end

def handle_info({:joined, _user} = msg, socket), do: Notifications.handle_info(msg, socket)
def handle_info({:left, _user} = msg, socket), do: Notifications.handle_info(msg, socket)

What do y'all think about adding macros to ChannelHandler for join and handle_info such that something like the following would be possible?

join "lobby", Lobby
join "room:" <> _id, Room

info {:joined, _user}, Notifications
info {:joined, _user}, Notifications

This would really help us streamline our Channel modules into pure "routers" and I imagine could be useful for other ChannelHandler users. I'm happy to do the work on this feature but would like to gauge the likelihood of something like it getting merged.

While evaluating our options, I prototyped a simple set of macros that do what we need and may be of further use in demonstrating the use case: Dispatcher. Check LobbyChannel and RoomChannel for example usage.

My implementation doesn't include things like scopes and plugs, which I think are awesome features. Furthermore, I'd rather contribute to an existing community effort than create another library.

Happy to chat more, and looking forward to hearing your thoughts.

@ngscheurich
Copy link
Author

Forgot to mention: As the join fn _topic, _payload, socket -> ... syntax is already supported, I'd just be looking to add the join _topic, ModuleName form.

@doorgan
Copy link
Owner

doorgan commented Aug 14, 2023

Hi @ngscheurich!

These changes would be welcome, feel free to submit a PR(I'd be happy to provide context if you're not familiar with Spark, the library this is built with), or I can do that when I get some time

Specifically, I think we need the following additions:

  1. Support info pattern, handler_module, function_name \\ :handle_info
  2. Support info function like we have handler for events. The reason for this is that the functions generated by this library are injected on a before_compile callback, so I'd rather have all the functions defined in a predictable order, so the clauses evaluate in the same order the user defined them
  3. Support join topic, handler_module, function_name \\ :join

I was also playing with the idea of having something like LiveView's on_mount and hooks to fully consolidate all info logic in the handler modules, but that's a topic of its own :D

@ngscheurich
Copy link
Author

Sounds great, @doorgan. Thanks for the pointers. I'll submit a PR and we can go from there!

@ngscheurich
Copy link
Author

@doorgan I've done a bit of work on this but am unsure how to proceed. I'm stuck on getting Spark to allow pattern matching on a Spark.Dsl.Entity arg. For instance, I've created an info entity like so:

@info %Spark.Dsl.Entity{
  name: :info,
  target: Info,
  args: [:message, :module, {:optional, :function, :handle_info}],
  schema: [
    message: [type: :any, required: true],
    module: [type: :atom, required: true],
    function: [type: :atom, required: true]
  ],
  modules: [:module]
}

I can use it in a router like this:

info {:info, :delegate}, ChannelHandler.RouterTest.TestHandler

The problem crops up, however, if I use try to pattern-match the message arg:

info {:info, _}, ChannelHandler.RouterTest.TestHandler

I'm reading over the Spark source but thought you might have some insight in the meantime.

@doorgan
Copy link
Owner

doorgan commented Sep 10, 2023

@ngscheurich Ah! You need to set the message type to :quoted
Spark uses NimbleOptions under the hood, but has special handling for some types, you can see the list of supported types in https://hexdocs.pm/spark/Spark.OptionsHelpers.html

So the idea is that you use that pattern to generate a handle_info function in ChannelHandler.Extension.handle_before_compile

@ngscheurich
Copy link
Author

Ah, great! Thanks for the pointer, @doorgan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants