search

phoenixframework / phoenix_live_view

Rich, real-time user experiences with server-rendered HTML
https://hex.pm/packages/phoenix_live_view
MIT License
6.18k stars 930 forks source link

Unexpectedly large diffs and documentation #522

Closed piisalie closed 4 years ago

piisalie commented 4 years ago

Thanks for all your hard work on LiveView. The team I am a part of has built a product over the last few months using almost all LiveView with great results. As a testament to it's functionality: we have apparently made many terrible architectural decisions in the process and LiveView continues to work. (in spite of 20kb json blobs being sent often every second)

I understand that temporary_assigns can be used to mitigate some of these issues, I'm more interested in gaining insight into what constitutes a good LiveView architecture in a real app and how we can better document that.

Environment

Actual behavior

We isolated a few cases where the LiveView diffing behavior was surprising and unexpected. It's not clear to me that it's a bug, but I would love to help improve the docs or functionality in such a way to prevent others from going down this same path.

:one: If an attribute of a Map is updated, every place that Map is used is updated.

I've created a small app recreating this here

If you visit to the / route, and then in a separate tab visit /send-message it will send a message updating the messages attribute of the Map. The diff sent to the socket looks like this:

null,null,"lv:phx-boyFzi8o","diff",{"0":"some place nice","1":{"d":[["3","three"],["1","first"],["2","second"]]}}]

Any time messages is updated, the title attribute is also sent down in the diff even though it didn't change.

The template looks like: https://github.com/piisalie/phoenix_liveview_oddities_and_misc/blob/5d5eeab3ae0b38dbf2228fd74a67d990d13e3379/lib/diff_liveview_repo_case_web/live/messages.ex#L7-L14

And the update code: https://github.com/piisalie/phoenix_liveview_oddities_and_misc/blob/5d5eeab3ae0b38dbf2228fd74a67d990d13e3379/lib/diff_liveview_repo_case_web/live/messages.ex#L34-L38

This was problematic for us when rendering multiple lists of items, or nested maps, belonging to the same struct, if one of the attribute lists was updated, all of the places that struct was used was passed down in the diff... which was slightly surprising.

:two: A content_tag that relies on socket assigns will send down all of the HTML in the diff (as opposed to just the value that changed).

Using the same app as above, if you visit the /update-thing route, it will update an assign that is used in a function using content_tag:

https://github.com/piisalie/phoenix_liveview_oddities_and_misc/blob/master/lib/diff_liveview_repo_case_web/live/messages.ex#L7-L14

https://github.com/piisalie/phoenix_liveview_oddities_and_misc/blob/5d5eeab3ae0b38dbf2228fd74a67d990d13e3379/lib/diff_liveview_repo_case_web/live/messages.ex#L44-L50

And updated via: https://github.com/piisalie/phoenix_liveview_oddities_and_misc/blob/5d5eeab3ae0b38dbf2228fd74a67d990d13e3379/lib/diff_liveview_repo_case_web/live/messages.ex#L40-L42

This resulting diff includes all of the HTML: 

[null,null,"lv:phx-boyFzi8o","diff",{"2":"<div><span>a new hope</span></div>"}]

This sadly can balloon the size of your diffs if you're unaware and using many nested functions / logic, as the whole string gets computed as dynamic and is sent down.

Expected behavior

:one: I would expect for only values of a Map/Struct that changed to get sent in the diff, or for more specific documentation around this behavior to prevent people like me from assigning all of the state to a struct. :upside_down_face:

The problem we were trying to solve with this approach was: how to easily test and manage a complex state. Building a data structure and functions to transform it made sense at the time :joy:.

:two: It would be great if we didn't send down giant strings of html :).... but I think we at least need some more clear documentation here. I understand why technically this is not something that's easily possible, but it wasn't until we started paying attention to diffs and had a large number of items rendered this way that we really realized it was problematic. At that point we were relying on nested content_tag functions quite heavily.

The problem we were trying to solve with this approach was: easy template sharing and rendering logic across views.

I would love to help with this but at this point I am not sure I fully understand what we want to advocate as "good Live View architecture" in more complicated applications. Hopefully we can expand on that a bit here. :)

Thanks again for everything <3

josevalim commented 4 years ago

Thank you! We can't fix those trivially, so we will update the docs. For content_tag, the best is to not use content_tag at all, and write out the HTML. I have heard there are some libraries coming out soon to make it easier to write the HTML out. :)