search

phoenixframework / phoenix_live_view

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

Regression: Anchor tags (eg `<a href="#">noop</a>`) dynamically added to the DOM from hooks trigger LiveView disconnects on click since `v0.18.4` #2394

Closed sherbondy closed 1 year ago

sherbondy commented 1 year ago

Environment

Actual behavior

Prior to Phoenix LiveView version 0.18.4, it was possible to dynamically add anchor tags, eg <a href="#">noop</a>, into a page via eg a Phoenix Hook, and clicking those links would not interfere with the parent LiveView page.

This is relevant if you are eg mounting React components or other dynamic anchor tags in a page via hooks.

Since 0.18.4, any clicks of dynamically generated links like this trigger LiveView disconnects, I believe due to the changes here: https://github.com/phoenixframework/phoenix_live_view/blame/fe07e999745eb91b4777e530788331f40e23d3c3/assets/js/phoenix_live_view/live_socket.js#L637

I'm guessing this was an accidental regression due to changes made to try and fix other issues relating to navigation in the last series of patch releases.

If this is a deliberate breakage and there are no plans to gracefully support (ie have phoenix liveview not interfere with) dynamically inserted anchor tags or sites that do a mix of client-side routing (eg for some react sub-components) and server-side routing => would it be possible to maybe ignore binding phoenix click handlers (eg in bindClicks) to anchor tags whose parent elements are set to phx-update="ignore" or otherwise, and likewise not trigger an unload() on the parent liveview/socket?

Single file app example that reproduces this issue: https://github.com/sherbondy/ahref_bug/blob/main/minimal.exs

Little video of repro: https://user-images.githubusercontent.com/193187/212289894-bc21be75-0f85-4b2b-b0e8-9443d348edac.mov

Expected behavior

My expectation is for LiveView to ignore any dynamically inserted anchor tags that aren't rendered by phoenix components / are not annotated with phx-click / opted into live-navigation via link => just fallback to default browser navigation behavior without disconnecting the parent liveview (as was the case prior to v0.18.3).

❤️

Thanks, otherwise haven't had a major hitch using LiveView in prod => doing patch version upgrades in a long time, it's been pretty seamless otherwise!

sherbondy commented 1 year ago

Thanks Chris for the quick fix! ❤️ 🔥 🐦 Will test it out and report back if any outstanding issues/regressions.

Not sure if this patch allows for a hybrid liveview/react page with a subset of navigation controlled by client-side routing to work but should seemingly resolve hash URLs triggering disconnects.

Mixing client-side routing and liveview routing within a single page is sort of asking for trouble I guess => but there are unfortunately some situations that warrant it given latency concerns & cases where you might not want to roundtrip to the server but you still want to change the URL via pushState for an easily bookmark-able/share-able page which encodes client navigation state in the URL.

sherbondy commented 1 year ago

Looks like this patch fixes hash URL navigation but the LiveView JS lib hijacks other link tags in the page which may be doing eg client-side routing if they don't have hash URLs.

TBF, I think this behavior was ambiguous to begin with, but is there any feasible way to differentiate between links that are Phoenix <.link>s (and thus opted into live routing / their click events being hijacked by LiveView JS) and other dynamically generated a tags that you want to have left alone in the page / self-manage in order to facilitate things like client-side routing when you really need it?

Just trying to get a sense of what is supportable / where to draw the line => understand if design constraints with LiveView force drawing a line in the sand and just punting on things like supporting client-side routing for a subset of elements on an otherwise LV-rendered page.

Even if it requires explicit annotation => I wouldn't mind being forced to add a data attr or whatever to just tell LiveView, "please ignore this anchor tag that I created, and don't bind any event handlers to this, I want to manage its behavior myself." WDYT @chrismccord ?

Sorry I should have included a non-hash URL example in a hook too for the repro => some minimal element example with client-side pushState implemented. Can add that to example repo too if it helps.

Also would be happy to share a PR with some tweaks around the explicit opt-out annotation idea for links via some data attr or what have you, if you think that's a sensible direction / don't see a foolproof way to deal with fiddly edge-case things like this.

chrismccord commented 1 year ago

If you want to handle click events that have an href yourself, then you can event.stopImmediatePropagation() in your handler. We bind at top level on window so you can stop the event from bubbling to LV

sherbondy commented 1 year ago

Great thanks for the guidance Chris, will try this!