Skip to content

Add a plugin system - #114

Open
botandrose wants to merge 6 commits into
mainfrom
plugins
Open

Add a plugin system#114
botandrose wants to merge 6 commits into
mainfrom
plugins

Conversation

@botandrose

@botandrose botandrose commented Feb 15, 2025

Copy link
Copy Markdown
Collaborator

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

Idiomorph.addPlugin({
    name: 'logger',
    beforeNodeAdded: function(node) {
        console.log('Node added:', node);
    },
    beforeNodeRemoved: function(node) {
        console.log('Node removed:', node);
    },
});

Benefits

  • Potentially slimming down core by extracting less-frequently-used options into plugins. For example, ignoreActive or ignoreActiveValue might be good candidates for this.
  • Allowing transparent composition of different callback-related concerns. Right now, all lifecycle callbacks are singletons. So if one wants to register multiple beforeNodeMorphed callbacks, for example, they'd have to manually compose the individual functions themselves.
  • Enabling experimentation and third-party integration outside of core. This could enable third-party npm packages to be published that provide an Idiomorph integration or modification, without requiring us to merge anything into core.
  • May be internally useful for core, for separating concerns.

Questions

  • How do we want the interface to be? Should we name it defineExtension to line up with its cousin, htmx?
  • Can this system support require, import, and <script src>?
  • What if we don't control the Idiomorph call itself? For example, Idiomorph is a hidden implementation detail of Turbo. Could a user of Turbo add a plugin to Idiomorph, even though it's buried within the guts of Turbo?

So, is this a good idea? What do we think about the API?

@botandrose botandrose changed the title [DRAFT] Proof-of-concept for a plugin system Add a plugin system Feb 16, 2025
@botandrose

Copy link
Copy Markdown
Collaborator Author

I've resolved the known issues, so I'm graduating this to a full PR, and its ready for review.

@botandrose

Copy link
Copy Markdown
Collaborator Author

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 callbacks. Below is the code currently in Turbo for beforeNodeMorphed:

  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:

  • The data-turbo-permanent implementation
  • The private #beforeNodeMorphed property is how Turbo is currently composing callbacks. This is used specifically for reloading <turbo-frame>s on morph (rather than morphing its contents), implemented elsewhere.
  • Dispatching a custom turbo:before-morph-element event for allowing Turbo's consumers to tap into the beforeNodeMorphed logic themselves. Another strategy for enabling composition, ultimately.

Now, in my actual app, I have implemented an optimization: data-turbo-version. If this custom attribute is present on the old and new element, and the value of this attribute is not newer in the new element, the morph is prevented. This is critical for keeping the app responsive even though we're constantly pushing huge documents down the wire. Here is the implementation for that:

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 turbo:before-morph-element event, but there aren't corresponding events for the other Idiomorph hooks.
Secondarily, it seems clear that Turbo is already struggling a bit with the lack of callback composition, even before I wanted to add in my own bit externally.

So, I submit that the plugin system I'm proposing would clean all this up nicely.

@1cg

1cg commented Feb 21, 2025

Copy link
Copy Markdown
Contributor

Two questions:

  • If we add this, would it make sense to remove the old callback system. We are pre-1.0 so now's the time to do it, and it seems like this replaces it.
  • What's the perf hit?

If the perf hit isn't too bad I'd prefer to remove the callbacks and just use this instead.

@botandrose

botandrose commented Feb 21, 2025

Copy link
Copy Markdown
Collaborator Author

@1cg Hmm yeah good point about the API considerations. If this were merged as-is, we would have three different methods of configuring Idiomorph:

  1. Idiomorph.defaults.merge({ ... })
  2. Idiomorph.morph(..., { callbacks: ... })
  3. Idiomorph.addPlugin({ ... })

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 idiomorph:beforeNodeMorphed et al events? I think they might be a bit better suited to composition.

@botandrose

Copy link
Copy Markdown
Collaborator Author

@1cg The more I consider the events idea, the more I like it.

  1. It would reduce the API surface area, not increase it
  2. Events are naturally composable

I'll explore it on its own branch, and we can compare.

@1cg

1cg commented Feb 21, 2025

Copy link
Copy Markdown
Contributor

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.)

@botandrose

Copy link
Copy Markdown
Collaborator Author

@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.

@1cg

1cg commented Feb 21, 2025

Copy link
Copy Markdown
Contributor

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 document which requires bubbling. I can imagine a situation where you have a bubbling event that allows document to hook in event listeners on the element in question, but that starts to get a little complicated.

Let's see what the perf looks like and go from there. Strongly favor events if at all possible though!

@MichaelWest22

Copy link
Copy Markdown
Collaborator

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

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
idiomorph-full-event-to-document.txt
idiomorph-one-event-to-document.txt

@botandrose

botandrose commented Mar 3, 2025 via email

Copy link
Copy Markdown
Collaborator Author

@nickchomey

nickchomey commented Aug 24, 2026

Copy link
Copy Markdown

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:

  1. Keyed matching. The iAPI has some optional SPA-like functionality via data-wp-each, and for better or worse, often uses data-wp-key attributes to identify each list item rather than normal ids. Without an id, matching is positional, so idiomorph reorder rebuilds rows instead of moving them.
    I noticed that you already have a fuzzy synthetic ids item in your Roadmap. So, the hope is that those could be folded into the plugin/extensibility effort. Datastar added such a synthetic id mechanism as well.
  2. Element move notifications. Pantry parks and identity moves fire no callback today, so if you track element lifetime with, e.g. a MutationObserver, a move looks exactly like remove+insert, which defeats the state preservation morphing is meant to provide.
  3. Range reservation. For reasons, my wp-each directive uses comment markers to delineate rows, owned by client state. So, when a region updates, I need to be able to treat an entire range/block that travels with its template anchor.

I prototyped all three as extra plugin hooks on top of this branch:

  • getIdentityToken(element) - a general version of the fuzzy synthetic ids idea. The host decides what identity means - an explicit key attribute, a token derived from content, whatever - with no DOM mutation. Datastar also added a synthetic-id pass before merging (they assign generated ids to anonymous elements).
  • beforeNodeMoved(node, after) / afterNodeMoved(node) - fired whenever an existing node is repositioned: parked in the pantry, or moved by identity matching. Today the add/remove callbacks don't fire for moves at all, so
    there is no way to tell a move apart from remove+insert.
  • reserveRange(newChild, oldParent, ctx) - lets the host claim an incoming node before normal matching by returning { anchor, start?, end? }: anchor is the old node corresponding to the incoming node (it gets morphed normally), and start/end delimit a contiguous run of old nodes that is kept intact and moved to follow the anchor atomically - their contents are neither morphed nor removed. The host must either claim the range or supply its contents inline, never both. Returning null falls through to normal matching.

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?

@botandrose

Copy link
Copy Markdown
Collaborator Author

@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.

@nickchomey

Copy link
Copy Markdown

Sounds good! Just ping me here when you're ready.

@nickchomey

nickchomey commented Aug 25, 2026

Copy link
Copy Markdown

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 wp-each, which is fairly antithetical to a hypermedia-first approach and introduces significant complexity). If that happens, I think the first and third changes I made above (keyed matching and range reservation) would probably not be needed. Yet, you had already planned some sort of synthetic keys and the range idea might be useful in some way as well.

Anyway, when youre ready, I'm happy to help shape this however I'm able

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

Successfully merging this pull request may close these issues.

4 participants