-
Notifications
You must be signed in to change notification settings - Fork 2
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
Implement Plugin API for Content package #120
Conversation
Amazing! This will be such an upgrade in flexibility 💪 I realize this is still WIP, but adding my thoughts to your questions.
I can't think of a lot of actual practical scenarios where we would've needed this per source, but there may be value in understanding what content type we're dealing with. It looks like content transforms still expect a key/value format. In an ideal world, we would be able to apply transforms to any content source, like an XML or HTML file. I realize that could get a little more hairy with abstraction, but it seems valuable to pass through some sort of context. You might still be actively working on this, but I also wonder if each plugin file should work independently as a plugin instead of the current process of converting transforms into plugins within What do you think?
Could this work in parallel to How might we separate concerns per package (
Agreed with you here re consistency of possibly wanting to just install |
100% Agreed! Sticking with the key/value format was an interim solution while js configs were being developed on another branch. That said, the I'm working on bringing the js config changes into this branch now, thinking the new syntax will look something along these lines: import { defineConfig } from "@bluecadet/launchpad";
import { mdToHtml, sanityToPlain } from "@bluecadet/launchpad/plugins";
export default defineConfig({
content: {
// ...
},
monitor: {
// ...
},
plugins: [
sanityToPlain({
path: 'foo.bar'
}),
mdToHtml({
path: 'foo.bar.foobar'
}),
]
});
Yeah, I'm not sure if ConfigManager is the right place for this to live, as that's only used for the CLI entrypoint, and I imagine we also want the plugins to work with the Node API too. Expanding on the idea of moving core to utils... it could live in utils as a base class for I guess the tricky part is figuring out how to reuse the logic from
I feel like I might be over-engineering this lol – lmk what you think. That reminds me: do you think CommandCenter has a role in Launchpad once the plugin API is implemented? I think adding an import { defineConfig } from "@bluecadet/launchpad";
import { execScript } from "@bluecadet/launchpad/plugins";
export default defineConfig({
// ...
plugins: [
// example: build the app after the `onUpdateContent` hook
execScript({
hook: 'onUpdateContent',
script: 'cd ../apps && npm run build'
}),
]
}); |
That looks pretty good! Trying to think of a few other scenarios: For content plugins, it makes sense to me that they would know what sources there are, or at least be able to parse the config and operate on files at various stages. Have you thought about what this might look like for monitor or other types of plugins? E.g. how would a plugin be able to interact with pm2? For example, a heartbeat plugin that restarts the app if there's no heartbeat coming in, or a plugin that can tap into our future http server to provide a show control API. That's where my OOP brain goes to singletons by default, like
You're right. My angle was the note above, where plugins may have to access the same config (e.g. to know where assets are saved). That could be solved with a central context though.
Interesting. Conceptually, it might make sense to isolate bootstrapping. Perhaps that would just be a utils class in itself (
Good question. My thinking for that class was that it serializes all commands centrally, so that it's easier to manage and debug major events like starting/stopping apps or updating content. With AM/PM and our older version of launchpad, it became really hard to control the flow of signals when using a control panel where users could update content, start/stop apps, etc. I spent a lot of time troubleshooting race conditions/stuck states. CommandCenter in itself is a pretty dumb class, but that's OK imo. I could see a world where we move to |
Ah I see what you mean. I think that could totally work. We could skirt the bootstrapping question for now by just passing the context how we're currently passing 'parentLogger', so Monitor/Content could create a new context if none are passed. That said, I think your question from earlier regarding separation of concerns between monitor and content brings up a good point. We'll probably want PM2 to be in the context for all monitor hooks, and not the content hooks, so maybe a general context doesn't work here? I'll do some more thinking around the PluginDriver to see if there's a clean way it could accommodate different contexts for different hooks.
Only real benefit I think would be simplicity and clarity in the API / documentation. Having both the plugin API and CommandCenter means two independent sets of hooks/events with different behaviors, which I could see being confusing. |
Made a couple updates in the last few commits:
Still working on the following:
|
Interesting. So in the config.js we would instantiate drivers for each type of context, right? It's not that we're passing in those contexts individually--that's happening within the Content/MonitorPluginDrivers? |
The config side of it remains largely unchanged, these changes mostly effect the implementation side. A base There's also some additional type safety to make sure only a hook with sufficient context can be called, so for example the "onContentFetched" hook would result in a type error if you tried to call it with just the On the config end, you can build your plugin as usual and just expect different context types depending on the hook you're in: export default defineConfig({
// ...
plugins: [
{
name: "demo-plugin",
hooks: {
someContentHook: (ctx, addtlArgs) => {
// CTX will have type `ContentHookContext`
},
someMonitorHook: (ctx, addtlArgs) => {
// CTX will have type `MonitorHookContext`
}
}
}
]
}) |
Oh nice! That would've been my exact question with the hooks. Seems like an elegant approach. |
🦋 Changeset detectedLatest commit: 1a95bc6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
a0c9010
to
dac39ac
Compare
ref: #15 #114
Steps taken so far:
PluginDriver
class that keeps references to all plugins, and handles the sequencing logiccontent/lib/plugins
content.contentTransform
config options.Some quick thoughts and open questions based on the plugin development so far:
onStartup
hook to be called when a user runsnpx launchpad
ornpx launchpad content
)launch-from-cli.js
, but for the node API it's unclear how that would work. This is somewhat related to the bullet above.Update 09/12: regarding the second bullet above, I misunderstood how running commands like
npx launchpad content
work. I thought it was only creating an instance ofLaunchpadContent
, but it's actually creating aLaunchpadCore
instance, then only calling theupdateContent
andshutdown
methods. For consistency once the plugin API is implemented, I wonder if it's worth rethinking this so that if a user installs just@bluecadet/launchpad-content
, they can still expect the startup/shutdown events?