Refactor link nav #3570
Refactor link nav #3570
Conversation
| @@ -20,11 +20,13 @@ defmodule Phoenix.LiveView.Session do | |||
| def authorize_root_redirect(%Session{} = session, %Route{} = route) do | |||
There was a problem hiding this comment.
Nitpick: should we just inline this function inside channel.ex, where it is called? It would allow us to be more consistent and have all of the mounting conditions in one place. Your call.
There was a problem hiding this comment.
I'm going to be a nag on when public API is now being removed post 1.0. Please don't.
There was a problem hiding this comment.
Those functions are not public API. They are all in modules which are moduledoc false. They don’t appear anywhere in hexdocs.
Co-authored-by: Steffen Deusch <steffen@deusch.me>
|
❤️❤️❤️🐥🔥 |
| @@ -52,11 +51,11 @@ defmodule Phoenix.LiveView.Route do | |||
| @doc """ | |||
| Returns the internal or external matched LiveView route info for the given uri. | |||
| """ | |||
| def live_link_info(endpoint, router, uri) when is_binary(uri) do | |||
| live_link_info(endpoint, router, URI.parse(uri)) | |||
| def live_link_info_without_checks(endpoint, router, uri) when is_binary(uri) do | |||
There was a problem hiding this comment.
This was a public API change that broke backwards compatibility in LiveView Native. Can you please be more mindful when making public API changes. This came after LiveView 1.0 and should have been locked down.
There was a problem hiding this comment.
The route module is @moduledoc false and always has been as far as I know:
There was a problem hiding this comment.
live_link_info_without_checks is used in the LiveViewTest module. Because I've been rejected on moving LiveView over to multi-format support I've had to reproduce the entire LiveViewTest suite and supporting modules in LVN. When these underlying functions change I have to chase those down. Or consumers upgrade LiveView and LVN isn't updated yet and something breaks
There was a problem hiding this comment.
I replied to the comment that this function is public. It isn’t.
I know you discussed about abstracting LVT but I was not part of that discussion. My suggestion is to submit a proposal, potentially with a proof of concept PR publicly, so at least it is all on record. I can’t comment otherwise on what I have not been a part of.
No description provided.