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

Feature: subscribe callback from ProxyComponent #57

Open
typhonrt opened this issue Jul 21, 2022 · 4 comments
Open

Feature: subscribe callback from ProxyComponent #57

typhonrt opened this issue Jul 21, 2022 · 4 comments

Comments

@typhonrt
Copy link

typhonrt commented Jul 21, 2022

Hi @rixo et al.

I'm working full time+ on a desktop app like UI library for Svelte and have now been testing w/ the latest Vite & HMR. Each "application" has an external JS class reference that mounts a Svelte component; think the outer application window / header bar. This external class controls closing the window and positional aspects. HMR currently works fine for children components of the main mounted Svelte component for the app window. However, for refreshes in the main mounted component for each app window I need to receive a specific callback in the outer JS application class to update the mounted element reference and component details and hook it up to the existing position store that is located in the external class.

I have followed the Svelte store subscriber mechanism in ProxyComponent adding a single subscribe method that returns an unsubscribe function. I am aware that the ideal contract of ProxyComponent doesn't add any methods as it passes through everything to the wrapped actual Svelte component. I would be more than glad to discuss alternatives.

I have verified that the changes I have made in my fork pass all tests and that this feature solves my problem. I haven't added any new tests yet or made a formal PR as I'd like to discuss these proposed changes:
https://github.com/typhonjs-svelte/svelte-hmr
https://github.com/typhonjs-svelte/svelte-hmr/commit/9cc56aebb3342071065b119a744c203478cbce68

Just for some background here is a dev snapshot overview that shows my desktop app UI library in action. Presently I'm developing this UI library on the Foundry VTT TTRPG platform, but also plan to make it generally available to Svelte community in the near future for any app dev purposes.
https://www.youtube.com/watch?v=yIc0fYh1YGM&t

@typhonrt
Copy link
Author

Thanks @rixo for the quick discussion on Svelte Discord server. Link to discussion here:
https://discord.com/channels/457912077277855764/999693674139090954

There is an undocumented hook API that I was unaware of that allows registration for direct callbacks for after / before refresh to specific components.. Gist:

const cmp = new MyComponent(...)

cmp.$$.on_hmr.push(
  cmpBeforeUpdate => {
    ...
    return cmpAfterUpdate => {
      ...
    }
  }
)

Located in svelte-hooks.
https://github.com/sveltejs/svelte-hmr/blob/master/packages/svelte-hmr/runtime/svelte-hooks.js

@rixo
Copy link
Collaborator

rixo commented Jul 22, 2022

Thanks @typhonrt for raising the issue. I'm happy that the preexisting experimental API solves your current need, and I believe that your return on experience, combined with what motivated me to add it in the first place, is enough argument that we should make the extra effort to document this and make it public API.

I'm reopening this issue to keep track of this topic.

@rixo rixo reopened this Jul 22, 2022
@typhonrt
Copy link
Author

typhonrt commented Jul 23, 2022

Most excellent. I'd be glad to keep in touch as this turns into a finalized API. I can also better describe my specific scenario if it helps regarding the UI lib I have put together. There is a bit of "convention" in my lib that might not have been clear in our previous discussion on how it works. If the main Svelte component mounted for the app exports the primary element as elementRoot with accessors turned on in the component this marks it as an "application shell" and the outer application class stores this element and also hooks it up to a "position" store that controls location on the screen and does a lot of fancy stuff like animation.

I was able to get things rather smooth though as after I detect this convention this is the resulting hookup w/ ProxyComponent (isHMRProxy does basic duck typing check on the component constructor name for Proxy<) :

// If Vite / HMR / svelte_hmr is enabled then add a hook to receive callbacks when the ProxyComponent
// refreshes. Update the element root accordingly and force an update to Position by resetting `parent`.
// See this issue for info about `on_hmr`:
// https://github.com/sveltejs/svelte-hmr/issues/57
if (isHMRProxy(svelteData.component) && Array.isArray(svelteData.component?.$$?.on_hmr))
{
   svelteData.component.$$.on_hmr.push(() => () => this.#updateApplicationShell());
}

A potential concern I could see with the current API is removing callbacks. It's convenient to use an anonymous function as with the current usage you need to chain functions for before even if you are only interested in responding to after callbacks. It might be better to separate before and after callbacks into different arrays, so that a single function can be passed in for either before or after as that will be easier to remove them. What that may look like is possibly on_hmr being an object / class with addBefore, hasBefore, removeBefore, addAfter, hasAfter, removeAfter methods all taking single functions. While my situation doesn't require removing the callbacks that I add having a basic API to make it explicit removes the burden on any consumers if the underlying implementation changes. IE on_hmr no longer being an array, etc.

Just brainstorming what a final API could look like above.

@tomprince
Copy link

It would definitely be helpful for a situation I'm in. I have some code that has a mix of svelte components and imperative code that generates a fancy layout. I've used this hook in spirit-island-builder/spirit-island-builder#202 to automatically re-run the imperative code when the component reloads. The (current) last commit of that PR also has some helpers to make using the hook nicer. (Currently, you can see the file with the helpers here, though that link will break after the PR merges).

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