-
Notifications
You must be signed in to change notification settings - Fork 31
Upgrade to LiveView 1.1 #243
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
base: main
Are you sure you want to change the base?
Upgrade to LiveView 1.1 #243
Conversation
Upgrade phoenix_live_view from ~> 1.0.18 to ~> 1.1.0 to support template resolution changes and new rendering optimizations. Add lazy_html dependency for HTML parsing in tests, required by LiveView 1.1's test infrastructure. Use override: true for phoenix_live_view as live_view_native_test_endpoint requires an older version (~> 1.0.0-rc.8).
Implement annotate_slot/4 callback in Template.Engine as required by Phoenix.LiveView.TagEngine behavior in LiveView 1.1. Update Diff.render call to use LiveView 1.1 signature (4 parameters instead of 3) in rendered_to_diff_string helper. Add LazyHTML handling in ClientProxy.init to convert LazyHTML structs to strings before parsing, as LiveView 1.1 returns HTML responses as LazyHTML structs instead of plain strings.
Add missing template resolution logic to ViewTree.deep_merge_diff that prevents template position collisions from breaking rendering when subsequent patches contain incompatible template positions. Phoenix LiveView 1.1 introduced a template sharing optimization where comprehensions reference templates by integer position. When merging diffs from different renders, these position references can collide. The fix resolves integer template position references to literal static part lists during the merge operation, preventing collisions. This brings the diff merging behavior in line with Phoenix LiveView 1.1's implementation by: - Adding module attributes @template, @Keyed, @keyed_count - Adding template resolution clause to deep_merge_diff/2 - Implementing resolve_templates/2 helper functions
Add test/live_view_native/test/diff_test.exs with 5 tests covering template resolution and diff merging behavior, matching Phoenix LiveView 1.1's test suite. Improve ViewTree.deep_merge_diff to properly handle keyed comprehensions: - Add dedicated clause for @Keyed maps that iterates through positions - Handle nil, integer, map, and [position, map] values in comprehensions - Properly handle keyed_count of 0 by replacing instead of merging - Add struct guard to resolve_templates to prevent attempting to enumerate struct fields All 372 tests pass. Test cases added: - "merges unless static" - basic diff merging - "resolves moved comprehensions" - template position collision fix - "no warning when keyed count is 0" - edge case handling - "ignores structs when resolving templates" - struct handling - "copies streams" - stream metadata preservation
Remove tests that LiveView 1.1 removed to maintain parity: - Remove 6 patch_id tests from view_tree_test.exs LiveView 1.1 moved these from tree_dom_test.exs (removed the entire patch_id describe block). LVN had kept these tests, but for parity with LiveView 1.1's test structure, they should be removed. - Remove merge_diff test from view_tree_test.exs This test now lives in diff_test.exs (added in previous commit). LiveView 1.1 removed it from tree_dom_test.exs. After these removals: - view_tree_test.exs: 108 lines, 4 tests (matches LiveView 1.1's tree_dom_test.exs) - diff_test.exs: 109 lines, 5 tests (matches LiveView 1.1's diff_test.exs) Note: The "consume-and-redirect" test was not added because it existed in LiveView 1.0 but LVN intentionally excluded it in their original port. Maintaining LVN's original design decisions. All 365 tests pass.
| end | ||
|
|
||
| describe "patch_id" do | ||
| test "updates deeply nested markup" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why were these test removed?
mix.exs
Outdated
| {:phoenix, "~> 1.7.21"}, | ||
| {:phoenix_view, "~> 2.0"}, | ||
| {:phoenix_live_view, "~> 1.0.18"}, | ||
| {:phoenix_live_view, "~> 1.1.0", override: true}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why the override?
Update live_view_native_test_endpoint to latest main which supports LiveView 1.1, removing the need for override: true.
|
FYI I'm running this branch directly and can't get the standard to render inside LVN go app. I can trace the execution all the way to the |
|
Looks interesting. Would be nice to get this merged and up and running. invested quite a bit of time into LVN integration. temporarely removed all traces to be able to carry on with dev but still interested in having LVN |
|
Hi @adiibanez, I actually did some more work on this to see if it was possible to actually do this tracking in a robust way (and in general to explore the question of whether it's possible solve this class of problems reliably): So the above takes a much more thorough approach than what I generated here (doing an embedded based mechanism to find analogous files, and doing comparisons between old and new upstream versions, and comparisions between old upstream and current lvn) , and as far as I can tell actually does a pretty good job. If you wanted to play with it, I'd be happy to help you get up and running (I'm Tom Clarke on elixir slack) |
**** Not verified in actual use, but all tests pass ****
Upgrade to Phoenix LiveView 1.1:
This brings LiveView Native's diff merging behavior in line with Phoenix LiveView 1.1's implementation.
Note that I didn't replace Floki for LiveView native tests because LVN requires case sensitive tags.
The new test files since 1.0 in liveview.
I replicated the first two, but didn't do the component ones (because we don't have the other replicated component tests).
Here's a more systematic comparison of where we stand with test similarity:
Note that we compare 1.0.18 to the state before my changes. 1.1.14 is compared against this PR