Plugin management page #24
Conversation
avidreder
commented
Nov 13, 2023
avidreder
commented
- Adds page to show/enable plugins
- Adds service for storing and managing plugin configurations
- Moves dev plugins to individual imports to allow them to be enabled/disabled
- Adds destroy implementation to plugins
- Adds return value to writeJSON
| @@ -3,6 +3,19 @@ | |||
| * All of these plugins have HMR enabled, regardless of whether you only have the plugins folder open | |||
| */ | |||
|
|
|||
| export const getDevPlugins = () => { | |||
| return [import('character-plugin'), import('stash-plugin'), import('poe-log-plugin')] | |||
| export const importDevPlugin = (pluginName: string) => { | |||
There was a problem hiding this comment.
I'm not sure we need to do this, importing the plugin doesn't really enabled and disable them. We can import the plugins and just selectively call start on the enabled ones. This list of plugins will always just be these 3 example plugins since the list is really just here for people to add the plugin they are working on locally but that will never be committed.
There was a problem hiding this comment.
The main reason to do this was that I need a way to get a handle on the module and it's entry so that I call it's methods on the plugin-settings-page. I'm sure there is another way to do this, but couldn't figure it out.
There was a problem hiding this comment.
You can't set a dynamic variable as import.
We may have to write the file to the system with the plugins we want to be in dev enviornment with HMR enabled. The HMR should regonize this and will rename the imports. This would be the way. Its a bit complicated but it should work, because the HMR is listening for file changes. We just have to trigger a file change with valid imports and everything should work.
There was a problem hiding this comment.
Its because the HMR changes the module names with static replacement.
import('character-plugin') would be => require('entry-2416da1sd') or something like this.
There was a problem hiding this comment.
FWIW HMR still worked with the solution of importing then one by one.
The switch statement that maps the stored plug-in configs to imports allows us to pseudo dynamically import.
There was a problem hiding this comment.
Yes, but its hardcoded. With my idea, we could use your plugin to dynamially enable and disable plguins in HMR without this hardcoded switch. What do you think?
There was a problem hiding this comment.
So, we would let your plugin to be static enabled and the others are controlled by your plugin
There was a problem hiding this comment.
Its basically this:
https://github.com/PoeStack/poestack-sage/blob/main/src/echo-app/bump-version.js
But the plugins writes this dynamically. And we could read every plugin within the folder echo-plugins and echo-plugin-examples.
But we have to ensure, that the package.json in echo-app has set all plugins as dependency, or the HMR can not resolve the plugins
There was a problem hiding this comment.
@C3ntraX I think I see what you mean, but I think I need a little more help with how that would be implemented. I think I need you to create a gist of how this would work, so I can understand the pattern better.
@zach-herridge a few options for this PR:
- Merge as is, and iterate on it
- I can revert the dev plugin enablement code, and merge just the production plugin bit, then follow up with the solution @C3ntraX mentioned
- Keep it open
I think no matter what I'll probably need some assistance in implementing the dynamic imports
| @@ -3,6 +3,19 @@ | |||
| * All of these plugins have HMR enabled, regardless of whether you only have the plugins folder open | |||
| */ | |||
|
|
|||
| export const getDevPlugins = () => { | |||
| return [import('character-plugin'), import('stash-plugin'), import('poe-log-plugin')] | |||
| export const importDevPlugin = (pluginName: string) => { | |||
There was a problem hiding this comment.
Yes, but its hardcoded. With my idea, we could use your plugin to dynamially enable and disable plguins in HMR without this hardcoded switch. What do you think?
| @@ -3,6 +3,19 @@ | |||
| * All of these plugins have HMR enabled, regardless of whether you only have the plugins folder open | |||
| */ | |||
|
|
|||
| export const getDevPlugins = () => { | |||
| return [import('character-plugin'), import('stash-plugin'), import('poe-log-plugin')] | |||
| export const importDevPlugin = (pluginName: string) => { | |||
There was a problem hiding this comment.
So, we would let your plugin to be static enabled and the others are controlled by your plugin
| @@ -3,6 +3,19 @@ | |||
| * All of these plugins have HMR enabled, regardless of whether you only have the plugins folder open | |||
| */ | |||
|
|
|||
| export const getDevPlugins = () => { | |||
| return [import('character-plugin'), import('stash-plugin'), import('poe-log-plugin')] | |||
| export const importDevPlugin = (pluginName: string) => { | |||
There was a problem hiding this comment.
Its basically this:
https://github.com/PoeStack/poestack-sage/blob/main/src/echo-app/bump-version.js
But the plugins writes this dynamically. And we could read every plugin within the folder echo-plugins and echo-plugin-examples.
But we have to ensure, that the package.json in echo-app has set all plugins as dependency, or the HMR can not resolve the plugins
| } else { | ||
| plugin.destroy() | ||
| } |
There was a problem hiding this comment.
I would make await plugin.destroy() to give the plugin the chacne to write data before it gets destroyed
There was a problem hiding this comment.
Okay I see. destory calls unregister Route. Within the call the plugin could write the unregister after Promise.resolve()
Do you delete the plugin afterwards? Then I would wait for the plugin, if not it would make no difference I guess?
There was a problem hiding this comment.
No, there isn't any deletion in this PR. I wouldn't delete anything except by "uninstalling" it, which isn't implemented in this PR
