Add a plugin system - #114
Conversation
|
I've resolved the known issues, so I'm graduating this to a full PR, and its ready for review. |
|
So here's the actual use-case that first got me thinking about a plugin system for Idiomorph: Composition (or lack thereof) of multiple beforeNodeMorphed = (currentElement, newElement) => {
if (currentElement instanceof Element) {
if (!currentElement.hasAttribute("data-turbo-permanent") && this.#beforeNodeMorphed(currentElement, newElement)) {
const event = dispatch("turbo:before-morph-element", {
cancelable: true,
target: currentElement,
detail: { currentElement, newElement }
})
return !event.defaultPrevented
} else {
return false
}
}
}As you can see, there are three separate concerns all mooshed together in there:
Now, in my actual app, I have implemented an optimization: if (currentElement.id && currentElement.hasAttribute("data-turbo-version") && currentElement.id === newElement?.id && currentElement.getAttribute("data-turbo-version") >= newElement?.getAttribute("data-turbo-version")) {
return false
}So, here's the motivating question: where do I put this code without forking Turbo or Idiomorph? I actually could most likely tap into Turbo's So, I submit that the plugin system I'm proposing would clean all this up nicely. |
|
Two questions:
If the perf hit isn't too bad I'd prefer to remove the callbacks and just use this instead. |
|
@1cg Hmm yeah good point about the API considerations. If this were merged as-is, we would have three different methods of configuring Idiomorph:
Let me think some more about how we might consolidate this API. Re: perf, I haven't looked into that at all yet, because the API is still up in the air. But maybe it would be informative to get some numbers at this point. As an alternative idea, what do you think about ditching callbacks entirely and going with dispatching |
|
@1cg The more I consider the events idea, the more I like it.
I'll explore it on its own branch, and we can compare. |
|
I very much prefer events, but didn't use them initially due to perf considerations. They have to bubble to be useful and bubbling means for a tree w/ N nodes you are going to trigger N*lg(N) event handlers w/N newly created events (iirc), w/fairly high constants as well. We can look at it again though: I definitely think that's the cleanest way to integrate independent pieces of software (see htmx, fixi, hyperscript, etc.) |
|
@1cg Ah yeah, that makes sense. We might be able to get around the perf issue by turning off bubbling and simply triggering them on the root node? Either way, with the new tachometer perf system, its super simple to compare the perf of branches to baseline. I'll spike all this out, and come back with some hard numbers. |
|
triggering it only on the root w/o bubbling is interesting, but in my experience the cleanest way to integrate things in general is to allow people to listen on Let's see what the perf looks like and go from there. Strongly favor events if at all possible though! |
|
I tested with dispatching events just to the I think the current callback solution is probably the most performant option but as Micha has pointed out it is not obvious on how to add complex composability with the callback. However technically callbacks are more customizable and composable than events it is just more complex to use in practice. Also from the use cases i've seen for datastore and turbo their is only one event that is really used in practice which is Idiomorph.defaults.callbacks.beforeNodeMorphed = (node) => {return document.dispatchEvent(
new CustomEvent("im-before-node-morphed", {
cancelable: true,
detail: { node },
}),
);};One idea I also had was to add a eventCallbacks optional string config value which is just a comma seperated list of callback names to emit as events so the end user can then choose just the one-two events they want to use event listeners for and take the perf hit just on these events while other users get no negative impact. So i mocked this up here: You just set the callback names and it emits the events as kebab case for compatiblity using Found a larger 20-40% slowdown with all events and bubbling in use idiomorph-full-event-bubbling.txt |
|
Wow, amazing work, Michael! Thank you for taking the time to explore this
so thoroughly. I've been in crunch mode trying to get bardtracker.com over
the line into beta, lately. But I'm very excited to take a look at the work
you've done in more detail!
…On Sun, Mar 2, 2025 at 10:16 PM MichaelWest22 ***@***.***> wrote:
I tested with dispatching events just to the document element and
attaching all listeners to document as well and it works just fine like
this and you don't then have to worry about bubbling. Also I found the
beforeNodeAdded event I had issues with when trying dispatching to the node
itself and using bubbling because the node is not yet on the DOM! Maybe you
can message the parentNode but this seems hacky and unreliable to me and I
think just sticking to document may be a good option to simplify things.
Avoids the support hassles when node trees lose event handlers during
morphing.
I think the current callback solution is probably the most performant
option but as Micha has pointed out it is not obvious on how to add complex
composability with the callback. However technically callbacks are more
customizable and composable than events it is just more complex to use in
practice. Also from the use cases i've seen for datastore and turbo their
is only one event that is really used in practice which is
beforeNodeMorphed as for almost all real world cases this is where the
most useful decisions can be made. So forcing all users to flood the
browser with all events that are never being listened to for every internal
action could be a bit of a waste. The current noop callbacks are probably
very efficient in this regard. I wonder if we could adopt a hybrid of some
kind where we keep the callback solution as it is now but provide the
ability for end users to if they choose select one or more of the existing
callbacks and convert it into a customEvent emitter. I think it should only
take a few lines of callback code to send the event to document and
return the result of the event back. This could just be a documented
recommended example callback pattern that turbo could then implement and
then they could move all the composable use cases to event listeners.
Idiomorph.defaults.callbacks.beforeNodeMorphed = (node) => {return document.dispatchEvent(
new CustomEvent("im-before-node-morphed", {
cancelable: true,
detail: { node },
}),
);};
One idea I also had was to add a eventCallbacks optional string config
value which is just a comma seperated list of callback names to emit as
events so the end user can then choose just the one-two events they want to
use event listeners for and take the perf hit just on these events while
other users get no negative impact.
So i mocked this up here:
main...MichaelWest22:eventCallback
<main...MichaelWest22:eventCallback>
You just set the callback names and it emits the events as kebab case for
compatiblity using beforeNodeMorphed -> im-before-node-morphed custom
event name. It works by just wrapping the existing callbacks with the
additional event listener so any existing callbacks in use keep working
100% as before to maintain full backwards compatibility. i ran perf tests
and found no real change if no events listed and the cost to call both the
custom event and the callback together did not slow things down more. But
adding the event dispatch did cause a large slowdown especially if we fire
it on all 7 events.
Found a larger 20-40% slowdown with all events and bubbling in use
Found all events but with no bubbling by using document it was maybe 5%
faster but still almost as slow
Found that just emitting the beforeNodeMorphed event to document was
around 10% slower
idiomorph-full-event-bubbling.txt
<https://github.com/user-attachments/files/19047301/idiomorph-full-event-bubbling.txt>
idiomorph-full-event-to-document.txt
<https://github.com/user-attachments/files/19047302/idiomorph-full-event-to-document.txt>
idiomorph-one-event-to-document.txt
<https://github.com/user-attachments/files/19047303/idiomorph-one-event-to-document.txt>
—
Reply to this email directly, view it on GitHub
<#114 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAEH36ACNIPWACWWWWMDCT2SPJTDAVCNFSM6AAAAABXGSPU26VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMOJTGIZTONJXGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
[image: MichaelWest22]*MichaelWest22* left a comment
(bigskysoftware/idiomorph#114)
<#114 (comment)>
I tested with dispatching events just to the document element and
attaching all listeners to document as well and it works just fine like
this and you don't then have to worry about bubbling. Also I found the
beforeNodeAdded event I had issues with when trying dispatching to the node
itself and using bubbling because the node is not yet on the DOM! Maybe you
can message the parentNode but this seems hacky and unreliable to me and I
think just sticking to document may be a good option to simplify things.
Avoids the support hassles when node trees lose event handlers during
morphing.
I think the current callback solution is probably the most performant
option but as Micha has pointed out it is not obvious on how to add complex
composability with the callback. However technically callbacks are more
customizable and composable than events it is just more complex to use in
practice. Also from the use cases i've seen for datastore and turbo their
is only one event that is really used in practice which is
beforeNodeMorphed as for almost all real world cases this is where the
most useful decisions can be made. So forcing all users to flood the
browser with all events that are never being listened to for every internal
action could be a bit of a waste. The current noop callbacks are probably
very efficient in this regard. I wonder if we could adopt a hybrid of some
kind where we keep the callback solution as it is now but provide the
ability for end users to if they choose select one or more of the existing
callbacks and convert it into a customEvent emitter. I think it should only
take a few lines of callback code to send the event to document and
return the result of the event back. This could just be a documented
recommended example callback pattern that turbo could then implement and
then they could move all the composable use cases to event listeners.
Idiomorph.defaults.callbacks.beforeNodeMorphed = (node) => {return document.dispatchEvent(
new CustomEvent("im-before-node-morphed", {
cancelable: true,
detail: { node },
}),
);};
One idea I also had was to add a eventCallbacks optional string config
value which is just a comma seperated list of callback names to emit as
events so the end user can then choose just the one-two events they want to
use event listeners for and take the perf hit just on these events while
other users get no negative impact.
So i mocked this up here:
main...MichaelWest22:eventCallback
<main...MichaelWest22:eventCallback>
You just set the callback names and it emits the events as kebab case for
compatiblity using beforeNodeMorphed -> im-before-node-morphed custom
event name. It works by just wrapping the existing callbacks with the
additional event listener so any existing callbacks in use keep working
100% as before to maintain full backwards compatibility. i ran perf tests
and found no real change if no events listed and the cost to call both the
custom event and the callback together did not slow things down more. But
adding the event dispatch did cause a large slowdown especially if we fire
it on all 7 events.
Found a larger 20-40% slowdown with all events and bubbling in use
Found all events but with no bubbling by using document it was maybe 5%
faster but still almost as slow
Found that just emitting the beforeNodeMorphed event to document was
around 10% slower
idiomorph-full-event-bubbling.txt
<https://github.com/user-attachments/files/19047301/idiomorph-full-event-bubbling.txt>
idiomorph-full-event-to-document.txt
<https://github.com/user-attachments/files/19047302/idiomorph-full-event-to-document.txt>
idiomorph-one-event-to-document.txt
<https://github.com/user-attachments/files/19047303/idiomorph-one-event-to-document.txt>
—
Reply to this email directly, view it on GitHub
<#114 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAEH36ACNIPWACWWWWMDCT2SPJTDAVCNFSM6AAAAABXGSPU26VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMOJTGIZTONJXGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Hi there, I noticed a comment that the v0.8.0 is set to be released imminently, but also notice that the ROADMAP says that plugins and various other features are to be part of 0.8.0. I presume that the Roadmap will be updated to bump those things to 0.9.0. Whatever the case, I would like to bump the discussion here as it relates to some blockers that I have just come across while trying to use idiomorph in a vdom-less rewrite of WordPress' Interactivity API. If you're not familiar, the iAPI has a lot of similiarities with Datastar, which ultimately forked idiomorph to be able to satisfy their own needs. I'd rather not maintain a fork, so hopefully the plugins mechanism can get implemented along with some related changes for extensibility. I rebased this PR on top of main and fixed a few conflicts, but three things came up that no current extension point can express:
I prototyped all three as extra plugin hooks on top of this branch:
The rebased PR + my additions is at main...nickchomey:idiomorph:plugin (in order to not have a bunch of rebase conflict markers in the diff, I ended up squashing the current 114 PR with the rebase conflict fixes plus my additions - not ideal, but it is what it is. I can change things if you really need it). It seems to suit my needs and nothing needed restructuring - the hooks go through the same callback chain as the existing ones, all tests are written against addPlugin only, and the full existing suite passes alongside them. Measured with your perf harness against main: no significant difference on any of the four benchmarks (all within ±3%) Of course, I have limited understanding of idiomorph, so have no illusions that this is the correct, or even an appropriate, solution. But I hope it can serve as a sketch for addressing these limitations if/when you return focus to this PR. I'm happy to discuss, change or test anything if it could be useful here. I'd also imagine that the Datastar maintainers would be happy to share useful insights about their fork's modifications. Would something like this fit better in this PR, or as follow-up plugin PRs once the base lands? |
|
@nickchomey Hi Nick, thanks for your comment here. First things first, you're right that this plugin system will be pushed past v0.8.0 to at least v0.9.0. v0.8.0 is intended to be mostly bugfixes and cleanup... the headline feature is probably simply publishing the types. However, I am very interested in working with you to figure out how the plugin system should work, since you have a concrete use-case. Once I get v0.8.0 out the door, let's revisit this in earnest. |
|
Sounds good! Just ping me here when you're ready. |
|
p.s. I should have made it clear that my effort related to the WP iAPI is more of a personal exploration, than anything sanctioned or planned by the api's maintainers. I'm fairly sure they will have zero interest in any of this and prefer to stick with the preact-based iAPI. Still, my aim is to use it personally and also offer it for free to plugin authors and site owners as a FOSS preact/vdom-free, hypermedia-first drop-in alternative, which would have plenty of extra functionality (eg inline js expressions, like datastar/alpinejs). Also, in the event that they don't want to adopt any of this, I may just remove all SPA-like functionality (like Anyway, when youre ready, I'm happy to help shape this however I'm able |
Overview
I think Idiomorph could benefit from a plugin system.
The idea here to is make it easy to encapsulate useful ideas and integrations in individual files or packages, e.g. "idiomorph/ignore-value", "idiomorph/htmx", "idiomorph/ignore-active-value", "idiomorph/turbo", etc. Multiple plugins should be able to be easily composed.
Example
Benefits
ignoreActiveorignoreActiveValuemight be good candidates for this.beforeNodeMorphedcallbacks, for example, they'd have to manually compose the individual functions themselves.Questions
defineExtensionto line up with its cousin, htmx?require,import, and<script src>?So, is this a good idea? What do we think about the API?