Allow to broadcast using poke / peek

[JanStevens]

Hello,

I see it in the code but its commented out, is this because poke_bcast does not work yet as intended or just a small mistake? poke_bcast would be very handy to have :)

Regards

[grych]

Intended.
Broadcasting the assign change may cause the unexpected side effects. You may have an expression which takes more than one assign as an argument, but you want to change the only one. The second assign value is taken from the browser which launched the event. This value may be different in every browser.

I left it commented out to think about it later. Maybe the good documentation would be enough to avoid confusion with the side effects?

[JanStevens]

Probably that would help, for me I don’t care about the current browser value or assigns since I just want to render a button that has it’s state returned from a Gen Server process. Would be cool if you make a release and uncomment it :D! Regards, - Jan Stevens

…

On 29 Sep 2017, at 12:10, Tomek Gryszkiewicz ***@***.***> wrote: Intended. Broadcasting the assign change may cause the unexpected side effects. You may have an expression which takes more than one assign as an argument, but you want to change the only one. The second assign value is taken from the browser which launched the event. This value may be different in every browser. I left it commented out to think about it later. Maybe the good documentation would be enough to avoid confusion with the side effects? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#60 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABeBg6ZCjWT6QtIWEhjQm4bCESnnWfYjks5snMIrgaJpZM4PodWU>.

[grych]

Still not convinced, broadcasting assigns may cause troubles.

Drab has the own assign cache (in Drab genserver), used when reading it with peek. When page loads, or there are changes on the page (like rendering templates), or changing the value of the assign, this cache is updated.

But if we allow broadcasted poke, there are two options:

1. All the connected browsers changes the assigns and sends them back to the server to update the cache. This would be a lot of data sent in the same moment, every time you are broadcasting poke
2. Browsers do not update the assigns cache. This means that all other browsers are in inconsistent state - peek returns something else than it is displayed on the screen.

I don't want to allow 2. This could be misleading, as you don't expect that behavior. All caches must be updated, but it must be done internally, not via browsers.

The issue is how to identify all Drab genservers to send them a message, which might be running on a different nodes, not even connected to each other?

I am leaving this issue opened, to be solved (or not) in a future.

[JanStevens]

I see two possible options:

1. What about explicitly marking an assign as broadcastable? This would mean that peek does not work on assigns marked as broadcast (preventing inconsistent state). It would only be changeable by using a broadcast. Mainly this is handy to indicate the overal state of some shared Genserver (ex: progress of a very long GenStage flow that every connected browser should see)

2. Another solution that could possible work is that in combination with option 1, broadcast caches are kept in a central place instead of a decentralized place. This actually makes sense and would allow to peek on broadcasted data without worrying on updating all the different drab genservers.

[grych]

1. Very interesting suggestion! That would solve all the problems. And with this, we don't have to introduce another function: poke could broadcast broadcastable assigns, and push others.
2. I've been thinking about it, but I think it would be overcompication. The way Drab genservers work now is very simple: they are created with start_link under the Phoenix channel, so in case of channel death (client is disconnected, etc), I don't have to care about cleaning up, etc. I just let it die. Very elixiry ;)

[JanStevens]

We could already start with 1 and then deal with 2 later on. I think it also deals with #18 were you already indicated that current setup with spawn link ends with zombie processes ( Cleary noticeable in dev mode with code reloading).

Keeping it simple is definitely important but an additional genserver to keep broadcast cache does not seem like an overkill imoh. I will see if I can help out with option 1 if you can give some small pointers 😃

[grych]

It will finally happen in the next release.
I completely redesigned the caching technique, now the cache is per-process (per event handler process). So inside the event handler process you live in a kind of the closure ;) - but of course there is a function to invalidate the cache.