Open fromkeith opened 1 year ago
Hi, thanks for opening this.
I totally see the need to bridge the gap from svelte on:* events to the host through an easy to use API. But I have a couple of considerations on whether or not this bridge should be in the injector and on the API design.
Since Svelte has not committed to make the on:*
feature available, having a similar feature in the injector could lead to creating an implementation that works while with the injector, but needs to be re-thinked when porting the parent component to Svelte.
This library is intended to be used as an helper while porting an application to Svelte, not as a long-term add-on to Svelte itself.
Having said this, the Svelte JS API has an $on
feature to do just that:
$on
component.$on(event, callback)
Causes the callback function to be called whenever the component dispatches an event.
The injector could make that API just a little bit closer and easier to reach from any other framework, in a more declarative fashion.
Having a single handle
function could be applicable to AngularJS but not to other frameworks or VanillaJS, where the function is not destructured as in AngularJS. Furthermore, having a reserved prop name is not really a solution.
A more vanilla oriented design could be having an on
field in SvelteElement that could automatically attach all the keys found in that object as an event, something like this:
<svelte-component component="component-name" on="$ctrl.on"/>
this.on = {
hello: (event) => {}, //hello handler
world: (event) => {}, //world handler
};
And the Injector would just glue them together like:
// On creation
Array.from(Object,entries(element.on)).forEach(([eventName, handler]) => {
// We need to keep the returned function for removing the event listener
element.off[eventName] = element.instance.$on(eventName, handler);
});
// On destroy
Array.from(Object,values(element.off)).forEach((removeListener) => {
removeListener();
});
In my opinion this is an ok API that could make it easier to bind events to functions, but:
on
field, would be ignored until the component is destroyed and mounted again. Making this reactive would require a lot of book keeping and more overhead still.As stated before, right now it is fully possible to use the Svelte JS API to attach events from a component instance:
$on
component.$on(event, callback)
Causes the callback function to be called whenever the component dispatches an event.
So in the svelte-injector world you would attach an onMount
listener to the component, and within the SvelteElement you get as the first (and only) parameter, you have the instance
field, which is exactly what Svelte gives back when binding an instance:
<svelte-component component="component-name" on-mount="$ctrl.svelteComponentMounted"/>
this.svelteComponentMounted = (element: SvelteElement) => {
// Svelte has mounted the component
element.instance.$on("hello", (event) => {
// I can handle the "hello" event
})
}
I personally have used this to attach events to already ported components while keeping the parent component in AngularJS.
Awesome thanks!
I was not aware of the element.instance.$on
, that makes way more sense to try take advantage of.
The nice thing about using a '&' binding in AngularJS instead of a '<' is that it puts it back into the AngularJS event loop. A direct callback from Svelte might not trigger AngularJS's change monitoring. I know in the past using 3rd party JS libraries I've had to use a $timeout
to get back into the event loop. Otherwise changes are not reflected in the UI.
Would something like this work?
<svelte-component component="component-name" onMount="$ctrl.svelteComponentMounted" onEvent="$ctrl.onEvent" subscribe="$ctrl.eventNames" ` />
Then inside the AngularJS svelte-component.js
(TODO: cleaner lambda + loop).
SvelteInjector.hydrate(rootElement, this.options).then(([element]) => {
this.element = element;
for (const name of this.subscribe) {
this.listenerHooks.push(this.element.instance.$on(name, (e) => this.onEvent({$event: e, name}));
}
});
Where subscribe
is binded by '<' and onEvent
is binded by '&'
(untested/compiled)
Alternatively... If you prefer the on="$ctrl.on"
syntax... (TODO: cleaner lambda + loop)
SvelteInjector.hydrate(rootElement, this.options).then(([element]) => {
this.element = element;
const eventNames = Object.keys(this.on);
for (const name of eventNames) {
this.listenerHooks.push(this.element.instance.$on(name, (e) => {
$timeout(() => this.onEvent({$event: e, name}))
});
}
});
Where on
is binded by '<'
(untested/compiled)
Or I can just update the README to include what you wrote in this ticket for using the onMount
callback...
Let me know!
Thanks for your response,
You're right, AngularJS has this special case where every event handler should be put back in its event loop to have some form of both-sides reactivity.
Maybe we could leave this helper to the Framework specific components, because:
This API is already able to attach events referencing the instance property of SvelteElement
(though it needs better docs), and adding a parameter to create
and would result in a poor experience because it already has multiple parameters (some with defaults).
Instead, the hydrate
function could create more than one component (since it looks for HTML elements with data-component-name), therefore setting the event listener on top of the function would mean attaching it to every component found, which could have different event signatures.
So for Framework specific components this would be:
For this case i would prefer something more like the second option, so you don't have to specify the subscribe array in another option, cause these two things should really be inferred from the listeners you provide.
And only svelte-component should care about AngularJS specific logic (wrapping events in $timeout).
// Inside $onInit
SvelteInjector.hydrate(rootElement, this.options).then(([element]) => {
this.element = element;
if(this.on) {
this.off = Array.from(Object.entries(this.on)).map(([eventName, listener]) =>
// Returns a function that removes the listener
element.instance.$on(eventName, (e) => {
// We here wrap our listener with the $timeout function for the AngularJS event loop
this.$timeout(() => {listener(e)});
})
)
}
})
// Inside $onDestroy
this.off.forEach((off) => off());
this.element?.destroy();
Where on
is nested by '<'.
<svelte-component component="my-component" props="$ctrl.props" on="$ctrl.on" />
this.on = {
hello: (e) => console.log(e.detail),
world: (e) => alert(e.detail),
};
Would this be alright?
A similar helper could be given to the React component too, without worrying about an additional event loop.
As we mentioned the VanillaJS API would need some better docs for event handling, and I can do that if you're not comfortable with it. As for component specific work, I feel that for now this would be the best way to go.
Thanks
Updated
Allow events from Svelte to propagate up to AngularJS. Theoretically possible for react as well, but I do not use react...
Basics
This issue in Svelte [1] talks about trying to get events on
svelte:component
. It looks like it is still active on discussion on what route to take. However, I find it necessary to pass events up from child components into the parent AngularJS environment. Taking a look at a comment in there [2] it is possible to do today. However, it involves usingsvelte/internal
packaging.This implementation follows that comment as a guide.
SvelteInjector Changes
handle?: (e: CustomEvent) => void;
toSvelteBaseElement
. Placeholder def so components can use it.angularjs/svelte-component.ts
is updated in thehydrate
promise return, to set thehandle
method to a callback. It pushes the event up through theonEvent
registered AngularJS Component.internal/InjectedComponent.svelte
is updated to incorporate [2]. It checks thehandle
is defined on thecomponent
.props
.Here is an example:
Some angular component.
MyComponent.svelte
Given above we would see the following logged to the console:
Limitations
onMount
it might miss child components dispatching events in theironMount
or earlier."on:":["hello"]
inprops
feels hacky, but it works...Links
[1] https://github.com/sveltejs/svelte/issues/2837 [2] https://github.com/sveltejs/svelte/issues/2837#issuecomment-1216541560