Skip to content
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

How to implement custom pages that need JS Hooks #455

Open
egze opened this issue Nov 8, 2024 · 14 comments
Open

How to implement custom pages that need JS Hooks #455

egze opened this issue Nov 8, 2024 · 14 comments

Comments

@egze
Copy link

egze commented Nov 8, 2024

I am implementing a package that provides a custom page for Live Dashboard and some things need to be done in JS as a Hook.

What is a good way to include hooks? Is it just a <script> tag in the heex template or there's a better location for them so that they can included in the assets?

@josevalim
Copy link
Member

It is just a script tag in the heex template, so it is loaded only when needed.

@SteffenDE
Copy link
Contributor

@josevalim this won't work as <script> tags dynamically added by morphdom are not executed. There's also no way to extend the hooks on the LiveSocket.

@SteffenDE SteffenDE reopened this Nov 8, 2024
@SteffenDE
Copy link
Contributor

@egze we have plans to make this possible with colocated hooks in a later version of LiveView.

@josevalim
Copy link
Member

So let's move this to LiveView, if not there yet? It is not specific to dashbard? :)

@SteffenDE
Copy link
Contributor

I'd say it is somewhat specific to LiveDashboard. We could make this possible without colocated hooks by allowing custom pages to implement a callback that is rendered inside the LiveDashboard layout's <head>. This could then render a script which add hooks like this window.hooks = {...(window.hooks || {}), ...myHooks} and then merge window.hooks when creating the LiveSocket. It doesn't apply to the usual LiveView application, as there you are in control of your assets.

@SteffenDE
Copy link
Contributor

I did something similar here:
main...SteffenDE:phoenix_live_dashboard:sd-before_closing_head_tag

and used in here: https://gist.github.com/SteffenDE/32955a42aa05f79be137f8165abbc8a7

Note that this does NOT use a real hook. It absolutely should though, as my workaround for the missing hooks is abysmal.

@josevalim
Copy link
Member

The application env unfortunately does not work, because different pages will want to customize the env. So we need to either ask the user or different custom pages will clobber each other?

I think we need a mechanism in LV morphdom that allows scripts inside the diffed content, and that will be treated as if we rendered a script on a regular page?

@SteffenDE
Copy link
Contributor

I didn't want to say that it should be implemented through the app env; it could also be an assign that is rendered in the layout. That's how I should have done it anyway. Stupid past me 😃

Executing scripts is tricky, because usually they don't handle it well when they are executed multiple times, which could happen quite easily when live navigating. So if we did support it, it should probably be opt-in like

<script phx-live>
  if (!window.myScriptLoaded) {
    // load
    window.myScriptLoaded = true;
  }
</script>

phx-live on a script could tell morphdom to execute the script, but the script must handle being executed multiple times itself.

@josevalim
Copy link
Member

But if we move them to the layout, then we need to do a whole page navigation, no?

Executing scripts is tricky, because usually they don't handle it well when they are executed multiple times, which could happen quite easily when live navigating. So if we did support it, it should probably be opt-in like

It depends. If we guarantee scripts do not have any interpolation in them, we can compute their MD5 and give them a compile-time fingerprint, which the client uses to not execute them again? We can also keep them linked to the current LiveView, such that, if the LiveView is remounted, we purge old scripts? Any global state will remain though...

@SteffenDE
Copy link
Contributor

But if we move them to the layout, then we need to do a whole page navigation, no?

yeah, you're right, my bad. Looks like past-me did actually have a reason not to use an assign. I was actually inspired by how ex_doc allows you to configure before_closing_head_tag.

The application env unfortunately does not work, because different pages will want to customize the env. So we need to either ask the user or different custom pages will clobber each other?

If dynamically added scripts are not evaluated, each page must render the same scripts anyway, so I don't think it's a problem if this was configured in the application env.

Any global state will remain though...

That's the main problem, yeah. But I think we'd be fine if the documentation clearly states that you must handle this.

@josevalim
Copy link
Member

The problem with application env is user convenience. It means that, for everyone who wants to add a custom page, they need to ask users to add it as a dependency and also change the application env. It would be great if it just worked.

@SteffenDE
Copy link
Contributor

Aaah I didn’t think about having pages from external sources. I was imagining only the user themselves adding their own ones.

We could also provide a function to register components to render in the head instead. Then anyone could do a

Phoenix.LiveDashboard.register_before_closing_head_tag(&MyModule.component/1)

and we store it for them somewhere (that could be app env, but also a process), it wouldn’t really make a difference then?

@josevalim
Copy link
Member

Yeah, we could do that, although I generally prefer to avoid side-effects like that, for the dashboard it may be fine. Although I still think we should support run-once scripts in LiveView :D

@SteffenDE
Copy link
Contributor

I'll open an issue in LiveView for supporting dynamically added scripts tomorrow. Maybe I'll also send a PR for LiveDashboard implementing what I described above. Then we can still see if we want to go with it, or wait for better support by LV.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants