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

plugin system for external npm dependencies #227

Merged
merged 17 commits into from
Jan 3, 2023
Merged

plugin system for external npm dependencies #227

merged 17 commits into from
Jan 3, 2023

Conversation

leastbad
Copy link
Contributor

@leastbad leastbad commented Nov 30, 2022

Type of PR (feature, enhancement, bug fix, etc.)

Enhancement?

Description

This PR introduces a general plugin store (Plugins) mechanism that can be easily accessed by new and existing operations.

// app/javascript/config/cable_ready.js
import consumer from '../channels/consumer'
import CableReady from 'cable_ready'
import morphdom from 'morphdom'

CableReady.initialize({ consumer, plugins: { morphdom } })
// OR
CableReady.initialize({ consumer })
CableReady.registerPlugin('morphdom', morphdom)

morphdom remains a dependency and is automatically registered as a plugin for v5 to ease transition.

The initial motivation was to create a path towards having access to newer alternatives to morphdom, including: idiomorph, micromorph, nanomorph, diffhtml, @alpinejs/morph and diffDOM, as well as likely more to come.

However, this implementation allows developers of custom CR operations to register any arbitrary library and access them via the same Plugins.foo mechanism.

I duplicated the morph operation to a new operation called morphdom, with the goal and intention of deprecating the morph operation in a future release. morph now raises a deprecation warning in the console, and SR will be updated to default to using morphdom as the morph operation.

If someone attempts to use an operation that requires a plugin which has not been registered, it will log a console error stating that the required plugin has not been registered but other operations will continue without interrupting the control flow.

Tasks

  1. Update SR to use the morphdom operation as the default morph operation instead of morph, including the example code in the initializer generator.
  2. Update the new SR/CR installers to add morphdom as a project dependency, and then register it as a plugin in their app/javascript/config/cable_ready.js as per the example above.
  3. Potentially create operations for idiomorph, nanomorph and @alpinejs/morph aka alpine_morph, pending discussion; we might simply offer code snippets in the docs instead of creating new operations. Update: will do this as a separate PR.

Why should this be added

  1. Zero dependencies + shave 2.1k off the bundle Decided to keep morphdom as a dependency for v5 for a smoother rollout and transition
  2. Less opinionated about morph libraries
  3. Reduce confusion with SR morph method

Checklist

  • My code follows the style guidelines of this project
  • Checks (StandardRB & Prettier-Standard) are passing
  • This is not a documentation update

@leastbad leastbad added enhancement proposal javascript Pull requests that update Javascript code labels Nov 30, 2022
@leastbad leastbad added this to the 5.0 milestone Nov 30, 2022
@leastbad leastbad self-assigned this Nov 30, 2022
@leastbad leastbad changed the title Proposal: plugin system + remove morphdom plugin system Jan 2, 2023
Copy link
Member

@marcoroth marcoroth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for exploring this idea!

I'm wondering if it would make more sense to rename the current morph operation to morphdom and rewrite the morph operation to delegate the operation call to the configured morph-library.

I was thinking something along the lines of:

import CableReady from "cable_ready"

CableReady.initialize({ morph: "morphdom" })

And then the morph operation is doing something like:

  morph: operation => {
    CableReady.operations[CableReady.config.morph](operation)
  }

This would also allow you to configure CableRady if don't want to use any morph-plugin at all:

CableReady.initialize({ morph: "innerHtml" })

@leastbad
Copy link
Contributor Author

leastbad commented Jan 2, 2023

Great suggestions, @marcoroth.

I like the idea that there could be a dominant morph method set. Lemme take a pass at seeing how I can make this work.

One thing I'm trying to decide is whether to refactor the current initialize method as you imply. It was created to pass the consumer to the web components. Right now I have a registerPlugin method that is called separately from initialize.

I suppose I already know the right answer, it's just a "do the work" thing. 😸

@leastbad
Copy link
Contributor Author

leastbad commented Jan 2, 2023

I am struggling with this in some ways, though.

I have a few goals:

  1. Ultimately transition morphdom from being a dependency
  2. Allow devs to use non-morphdom libs
  3. Allow devs to use multiple libs in different scenarios
  4. Remove confusion when there is an SR morph method and a CR morph operation
  5. Pop up a deprecation warning when the morph method is used in v5

Currently, I feel like the best plan for someone using CR is to be explicit in their choice of operation. Having to configure a favorite introduces a factor for someone else to change this value and mess everything up, and for not a lot of value. Trying to explain this scheme might prove quite challenging, too.

Also, having these pointers to other methods creates a lot of potential confusion wrt the events that are raised. If you call a morph operation, should it raise before-morph or before-idiomorph?

I think I would be a lot happier with someone just calling the operation that they want, and getting back the events that they expect. We've already done the work to support this on the SR side, via the :morph_operation setting. I am also very excited to eventually wave goodbye to the morph operation because we have had to provide support to people who are confused about the morph method/operation naming scheme.

Still thinking through everything.

@marcoroth
Copy link
Member

Right now I have a registerPlugin method that is called separately from initialize.

While I don't think having registerPlugin being called separately is an issue, I feel like the plugin naming is confusing, because a plugin could be anything. I feel like initialize makes more sense, since this is something you usually wouldn't want to change after you initialized it. Similar to how we have the initialize function in SR.

Currently, I feel like the best plan for someone using CR is to be explicit in their choice of operation.

Yeah I think that makes sense, but on the other hand we should let it default to something if they don't want to decide which morph library to use.

Also, having these pointers to other methods creates a lot of potential confusion wrt the events that are raised. If you call a morph operation, should it raise before-morph or before-idiomorph?

In this case it could call both. First before-morph, then before-idiomorph, after-idomorph and finally after-morph.
But we could also think about having the morph operation for transition-reasons and to remove it in CR 6.

On the SR side, :morph_operation could default to morph/morphdom and change to some other lib in SR4.

@leastbad
Copy link
Contributor Author

leastbad commented Jan 2, 2023

While I don't think having registerPlugin being called separately is an issue, I feel like the plugin naming is confusing, because a plugin could be anything. I feel like initialize makes more sense, since this is something you usually wouldn't want to change after you initialized it. Similar to how we have the initialize function in SR.

I'm not sure that I follow; the intention of the Plugins mechanism is that it can be anything. If you look at the README, I tried to explain:

However, this implementation allows developers of custom CR operations to register any arbitrary library and access them via the same Plugins.foo mechanism.

What this means is that someone could distribute a package of CableReady custom operations, and those operations could call upon Plugins so that they don't have to bundle every library those operations require. For example, someone could release a package of audio operations, which each use different audio-related npm packages.

Plugins was created with morph in mind, but I realized that it was more generally useful!

Yeah I think that makes sense, but on the other hand we should let it default to something if they don't want to decide which morph library to use.

morphdom ships with v5 and will be "registered" by default. The installer will add the boilerplate to register morphdom as a plugin; this means that when v6 eventually arrives, everyone's setup will continue working with morphdom unless they change it. The boilerplate also provides instruction on how to register other Plugins.

In this case it could call both. First before-morph, then before-idiomorph, after-idomorph and finally after-morph. But we could also think about having the morph operation for transition-reasons and to remove it in CR 6.

That's what I've done. morph operation is still there, it called the morphdom plugin and emits morph events. It's almost identical to the "new" morphdom operation, except that it also spits out a console warning.

I believe that our goal should be to nudge people away from using a morph operation. So long as there is a morph operation and an SR morph method, there will be people who are justifiably confused.

On the SR side, :morph_operation could default to morph/morphdom and change to some other lib in SR4.

After this PR is merged, SR will default to :morph_operation = :morphdom. So... mission accomplished!

@leastbad leastbad changed the title plugin system plugin system for external npm dependencies Jan 3, 2023
@leastbad leastbad merged commit f26b77b into master Jan 3, 2023
@leastbad leastbad deleted the morph_plugin branch January 3, 2023 21:45
@marcoroth marcoroth mentioned this pull request Jan 16, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement javascript Pull requests that update Javascript code proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants