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

feat: plugin API #252

Open
1 task
catuhana opened this issue Aug 10, 2023 · 6 comments
Open
1 task

feat: plugin API #252

catuhana opened this issue Aug 10, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@catuhana
Copy link

catuhana commented Aug 10, 2023

What do you want to see?

Currently, Plugin API doesn't support loading(?)/unloading when plugin is async. Async support for both plugins and onUnload function will fix the plugin unloading, and might make it easier to handle some asynchronous situations.

Revite Async Support Patch I already made a quick patch about it, but since I'm not familiar with Revite's technologies, I thought it would be better to both ask for the feature if it's in scope, and get a review about the patch. If it's considered to be added and patch looks fine, I can create a PR.
diff --git a/src/mobx/stores/Plugins.ts b/src/mobx/stores/Plugins.ts
index e6b3bbb3..eb4c50a2 100644
--- a/src/mobx/stores/Plugins.ts
+++ b/src/mobx/stores/Plugins.ts
@@ -58,7 +58,7 @@ type Plugin = {
 
 type Instance = {
     format: 1;
-    onUnload?: () => void;
+    onUnload?: () => Promise<void> | void;
 };
 
 // Example plugin:
@@ -140,7 +140,8 @@ export default class Plugins implements Store, Persistent<Data> {
     init() {
         if (!this.state.experiments.isEnabled("plugins")) return;
         this.plugins.forEach(
-            ({ namespace, id, enabled }) => enabled && this.load(namespace, id),
+            async ({ namespace, id, enabled }) =>
+                enabled && (await this.load(namespace, id)),
         );
     }
 
@@ -148,19 +149,19 @@ export default class Plugins implements Store, Persistent<Data> {
      * Add a plugin
      * @param plugin Plugin Manifest
      */
-    add(plugin: Plugin) {
+    async add(plugin: Plugin) {
         if (!this.state.experiments.isEnabled("plugins"))
             return console.error("Enable plugins in experiments!");
 
         const loaded = this.getInstance(plugin);
         if (loaded) {
-            this.unload(plugin.namespace, plugin.id);
+            await this.unload(plugin.namespace, plugin.id);
         }
 
         this.plugins.set(`${plugin.namespace}/${plugin.id}`, plugin);
 
         if (typeof plugin.enabled === "undefined" || plugin) {
-            this.load(plugin.namespace, plugin.id);
+            await this.load(plugin.namespace, plugin.id);
         }
     }
 
@@ -169,8 +170,8 @@ export default class Plugins implements Store, Persistent<Data> {
      * @param namespace Plugin Namespace
      * @param id Plugin Id
      */
-    remove(namespace: string, id: string) {
-        this.unload(namespace, id);
+    async remove(namespace: string, id: string) {
+        await this.unload(namespace, id);
         this.plugins.delete(`${namespace}/${id}`);
     }
 
@@ -179,14 +180,14 @@ export default class Plugins implements Store, Persistent<Data> {
      * @param namespace Plugin Namespace
      * @param id Plugin Id
      */
-    load(namespace: string, id: string) {
+    async load(namespace: string, id: string) {
         const plugin = this.get(namespace, id);
         if (!plugin) throw "Unknown plugin!";
 
         try {
             const ns = `${plugin.namespace}/${plugin.id}`;
 
-            const instance: Instance = eval(plugin.entrypoint)();
+            const instance: Instance = await eval(plugin.entrypoint)();
             this.instances.set(ns, {
                 ...instance,
                 format: plugin.format,
@@ -207,14 +208,17 @@ export default class Plugins implements Store, Persistent<Data> {
      * @param namespace Plugin Namespace
      * @param id Plugin Id
      */
-    unload(namespace: string, id: string) {
+    async unload(namespace: string, id: string) {
         const plugin = this.get(namespace, id);
         if (!plugin) throw "Unknown plugin!";
 
         const ns = `${plugin.namespace}/${plugin.id}`;
         const loaded = this.getInstance(plugin);
         if (loaded) {
-            loaded.onUnload?.();
+            const entrypoint = loaded.onUnload;
+            if (entrypoint?.constructor.name === "AsyncFunction")
+                await entrypoint?.();
+            else loaded.onUnload?.();
             this.plugins.set(ns, {
                 ...plugin,
                 enabled: false,
PWA
  • Yes, this feature request is specific to the PWA.
@catuhana catuhana added the enhancement New feature or request label Aug 10, 2023
@Rexogamer
Copy link
Member

We're currently rewriting the frontend and the plugin API will likely change significantly in the process; I'll transfer this so we know to keep it in mind when we get to that point

@Rexogamer Rexogamer transferred this issue from revoltchat/revite Aug 10, 2023
@catuhana
Copy link
Author

We're currently rewriting the frontend and the plugin API will likely change significantly in the process; I'll transfer this so we know to keep it in mind when we get to that point

That's great, but as far as I know, there's no ETA for both new client and plugin API. So maybe adding support to Revite as a quick patch a great idea, until the new client arrives?

@Rexogamer
Copy link
Member

There aren't any ETAs, correct. We're currently only doing important bug fixes for Revite/small tweaks that don't carry much risk - in my opinion, something like this would need more testing than we intend to do for Revite.

@insertish insertish changed the title feature request: async support for plugin API feat: plugin API Dec 1, 2023
@insertish
Copy link
Member

Hijacking this issue, proposal for new plugin format:

---
name = "Name"
author = "me"
---

// code

return {
  onSomeHandler: () => {}
}

TOML frontmatter on top of JS
This will be executed within a function and the return will be a set of event handlers to be registered, and later cleaned up.

@catuhana
Copy link
Author

catuhana commented Dec 1, 2023

I don't know if this is an already accepted proposal, but here's my recommendations:

  1. Instead of frontmatter styled plugin info, why not using a JSDoc solution, like TamperMonkey and such does, or having them to configure programmatically?

  2. AFAIK, top level returns aren't a thing. What about export { onSomeHandler: () => {} }, or having a function or class type (which will return the hooks and other useful data like plugin name etc.) and exporting it, similar to this.

@insertish
Copy link
Member

wrt (2), the idea would be that the plugin is implicitly wrapped in a function when evaluated, and then we can take the return at the end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: 🙋 Upvoting
Development

No branches or pull requests

3 participants