Wrapper function for creating plugins

[anikethsaha]

Another v5 feature we should consider is tweaking the docisfy plugin architecture to be more resilient. This warrants further discussion, but here are two ideas worth considering:

1. Provide a docisfy plugin "starter" or a sample repo to use as a starting point.

   This would make life much easier for the community and help enforce acceptance criters (below).

2. Define acceptance criteria for all docsify plugins.

   This is critical for docsify because unlike static sites, a docsify-powered site requires Javascript to render content. If a docsify plugin throws an error, it is almost certain that site functionality will be broken.

   Here are a few idea for acceptance criteria:

   • Specify the version of ECMAScript plugin developers should target. For docsify v4.x, ES5 is the required target to ensure IE11 support. This means dev must either author their plugin with ES5 syntax or transpile their code from ES6+ to ES5. For docsify v5, we'll have to look at our supported browser list to determine the appropriate ECMAScript version to target.
   • Plugin code must be wrapped in a try { ... } catch(err) { ...} statement to prevent a plugin error from breaking a site.
   • Plugins must make their version number accessible to other plugins for interoperability. Docsify addons like docsify-themeable and docsify-tabs already do this by adding an object to the global $doscify object ($docsify.tabs and $docsify.themeable) with a value key and value. The goal is to allow both docsify and other plugins to determine which plugins are active and, if necessary, modify their behavior as needed.
   • Plugins must specify a compatible docsify version to prevent breaking sites when a new version of docsify is released. For example, consider a v4 site that is configured to load the latest version of docsify and multiple plugins. If docsify v5 is released using the same URLs, the v4 site will automatically be "upgraded" to v5 but will likely break as a result of trying to load v4 plugins. This safeguard would allow docsify (or the plugin itself) to determine if it is safe to run the plugin before doing so, which would at least allow the site to load and function properly (albeit without incompatible plugin functionality). This wouldn't be necessary if users locked their docsify, theme, and plugin CDN URLs to a major version (see Specify major version number in URLs #780), but since they don't a safeguard like this is necessary.

Just some ideas to get started.

Originally posted by @jhildenbiddle in #1061 (comment)

[anikethsaha]

@jhildenbiddle this is really an awesome suggestion. It would make the plugin integration much more strong and simple.

May be we should extract this suggestion into a new issue.

My 2 cents ⬇️

Also, I am thinking of creating a wrapper function something like this createDocsifyPlugin function that needs to be returned from the plugin. It will accepts the following data

interface PluginMetaI {
  version: String;
  docsifyVersion: String;
  polyfillURL?: String;
  definition(hook: Object): void;
  pluginDocsURL?: string;

  // SOme more meta datas,/
}

const createDocsifyPlugin = (PluginData: PluginMetaI): Promise<void> => {
  const { version, docsifyVersion, polyfillUrl, definition } = PluginData;

  // some validation of the datas
  // like loading the polyfill

  // ...

  return new Promise((res, rej) => {
    try {
      res(definition(hook));
    } catch (err) {
      rej(err);
    }
  });
};

export default createDocsifyPlugin({
  version: "4",
  docsifyVersion: "4.11.0", //minimum this version is required.
  definition: hook => {
    hook.beforeEach(function() {});
    // ...
  }
});

createDocsifyPlugin will be a global function I guess so that inline plugins can be returned as well.
I used promise so that we can handle the async and errors better.

This is just an example I am thinking of the implementation. We can discuss this for a better approach.

---

From #1061 (comment)

[jhildenbiddle]

Good stuff, @anikethsaha.

The polyfillURL property concerns me a bit but perhaps this is because I'm not aware of how you're intending it to be used. For example: Do we really want plugins to load polyfills? What good is a polyfill URL without a test to determine if the ponyfill needs to be loaded? Why one URL [String] instead of multiple [Array]? What if multiple plugins try to load the same polyfill? You get the idea.

Regarding the wrapper function, if we move from Function-based plugins to Object-based plugins we can wrap each plugin without having to expose or require a call to a global function. This would add only a minimal amount of complexity for plugin authors and give maintainers additional control over how plugins are handled (like wrapping the definition() call in a try/catch block).

Instead of doing this as users do today:

window.$docsify = {
  plugins: [
    function(hook, vm) {
      // ... 
    }
  ]
};

Users would do something like this:

window.$docsify = {
  plugins: [
    {
      version: "4", // required
      docsifyVersion: "4.11.0", // required
      definition(hook) {
        // ... 
      }
    }
  ]
};

Plugin authors intending to distribute their code would be free to add additional properties as needed (based on docsify's own list, or perhaps custom properties used by the plugin author for interoperability with other plugins):

window.$docsify = {
  plugins: [
    {
      name: "My Plugin",
      author: "John Smith",
      description: "Does stuff!",
      homepage: "http://myplugin.com",
      version: "4",
      docsifyVersion: "4.11.0"
      definition(hook) {
        // ... 
      }
    }
  ]
};

This could also be a good way to prevent v4 plugins from being invoked by docsify v5: When iterating over items in the plugins array, ignore anything that is not a POJO with specific properties (e.g. version, docsifyVersion, and definition).

[anikethsaha]

The polyfillURL property concerns me a bit but perhaps this is because I'm not aware of how you're intending it to be used. For example: Do we really want plugins to load polyfills? What good is a polyfill URL without a test to determine if the ponyfill needs to be loaded? Why one URL [String] instead of multiple [Array]? What if multiple plugins try to load the same polyfill? You get the idea.

polyfillURL was a rough idea, I was hoping if we can load that link from docsify core at the end of the mounting of all datas/js. Like if two plugins are requesting the same polyfill, we can load a single one (I guess browsers does that already, not sure. ). Caching can be done as well.
Its not a property we surely need now, we can discuss over that .

Users would do something like this:

window.$docsify = {
  plugins: [
    {
      version: "4", // required
      docsifyVersion: "4.11.0", // required
      definition(hook) {
        // ... 
      }
    }
  ]
};

Yea we can have like this as the goal is to somehow wrap the plugin with our wrapper function, either it is done by the plugin author or by the core, so +1 for 🔼

Plugin authors intending to distribute their code would be free to add additional properties as needed (based on docsify's own list, or perhaps custom properties used by the plugin author for interoperability with other plugins):

agreed.

I wouldn't think much of backward compatibility of how to write plugins or config as we are shipping breaking change with major release.

This could also be a good way to prevent v4 plugins from being invoked by docsify v5: When iterating over items in the plugins array, ignore anything that is not a POJO with specific properties (e.g. version, docsifyVersion, and definition).

yes

[trusktr]

Let's also think about what the use case for plugins is.

Currently, the top use case seems to be for injecting HTML markup into the markdown output. How can we make that easier (whether its with plugins, or something else)? Can we provide more hooks, f.e. hook for before/after paragraphs, before/after headings, etc? Could we also provide a special syntax that basically is an insertion point that a plugin can hook into?

I suppose there's two things here:

1. improve how plugins are provided (objects allows more info to be provided)
2. improve what plugins can do

Now that I think about it, maybe the second point is better for a new issue, while this issue doesn't provide plugins new abilities, but just a better way to keep track of them and to allow them to, for example, easily have their own state within the object (methods can access any state using this, and plugins can be instantiated from classes too):

class SomePlugin {
      version = "4", // required
      docsifyVersion = "4.11.0", // required
      foo = 123
      definition(hook) {
        this.logFoo()
      } 
      logFoo() { console.log(this.foo) }
        // ... 
}

window.$docsify = {
  plugins: [
    new SomePlugin(),
    // ...
  ]
};

[trusktr]

This makes me think: what if instead of passing hook with all the hooks, we call the same-name methods of the POJO?

window.$docsify = {
  plugins: [
    {
      version: "4", // required
      docsifyVersion: "4.11.0", // required
      init(vm) {
        // ...
      } 
      beforeEach(vm) { ... }
      mounted(vm) { ... }
    }
  ]
};

or equivalently

class SomePlugin {
      version = "4", // required
      docsifyVersion = "4.11.0", // required

      init(vm) {
        // ...
      } 
      beforeEach(vm) { ... }
      mounted(vm) { ... }
      // ... 
}

window.$docsify = {
  plugins: [
    new SomePlugin(),
    // ...
  ]
};

[jhildenbiddle]

what if instead of passing hook with all the hooks, we call the same-name methods of the POJO?

One advantage of passing hook to a single function is that sharing data between methods is straightforward:

{
  version: "4", // required
  docsifyVersion: "4.11.0", // required
  definition(hook) {
    const myVar = 1;

    hook.init(() => {
      // Do something with myVar...
    });
    hook.beforeEach(content => {
      // Do something with myVar...
    });
    hook.afterEach((html, next) => {
      // Do something with myVar...
    });
  }
};

If hook methods are called directly on the plugin object itself, sharing data becomes more complex because there are multiple ways to do it (settings globals, using localStorage or sessionStorage, adding data to the object and referencing it with this, etc):

{
  version: "4", // required
  docsifyVersion: "4.11.0", // required
  onInit() {
    const myVar = 1;
  },
  onBeforeEach(content) {
    // Do something with myVar...
  },
  onAfterEach(html, next) {
    // Do something with myVar...
  }
};

Just something to think about. I like the cleanliness of having hook methods directly on the plugin object but I much prefer the simplicity of how data is shared between hook methods in the first example.

[trusktr]

simplicity of how data is shared between hook methods in the first example.

I think using this is as simple:

{
  version: "4", // required
  docsifyVersion: "4.11.0", // required
  onInit() {
    this.myVar = 1;
  },
  onBeforeEach(content) {
    // Do something with this.myVar...
  },
  async onAfterEach(html) {
    // Do something with this.myVar...
  }
};

(Sidenote, the example uses async functions instead of callbacks (instead of next))

Using this, the object can be more easily extended, and we can use super to call parent methods, and read parent state, etc, which I think is a nice benefit:

class SomePlugin {
  version = "4" // required
  docsifyVersion = "4.11.0" // required
  onInit() {
    this.myVar = 1;
  }
  onBeforeEach(content) {
    // Do something with this.myVar...
  }
  onAfterEach(html, next) {
    // Do something with this.myVar...
  }
};

class SomePlugin2 extends SomePlugin {
  onInit() {
    super.onInit()
    this.otherVar = 1
  }
  onBeforeEach(content) {
    const md = super.onBeforeEach(content)
    // Do something with this.otherVar...
  },
  async onAfterEach(html) {
    const html = await super.onAfterEach(html)
    // Do something with this.otherVar...
  }
}

If we want private variables (like you get with function-scoped variables, then we have private fields:

class SomePlugin {
  version = "4" // required
  docsifyVersion = "4.11.0" // required
  #myVar
  onInit() {
    this.#myVar = 1;
  }
  onBeforeEach(content) {
    // Do something with this.#myVar...
  }
  onAfterEach(html, next) {
    // Do something with this.#myVar...
  }
};

class SomePlugin2 extends SomePlugin {
  onInit() {
    super.onInit()
    console.log(this.#myVar) // error, not possible to access parent private property.
  }
}

(Private properties work in Chrome and Edge out of the box today.)

For comparison, here's the equivalent code to achieve variable sharing and extensibility if we go the single-function approach, which I think is less ideal:

class SomePlugin {
  version = "4" // required
  docsifyVersion = "4.11.0" // required
  definition(hook) {
    hook.init((...args) => this.onInit(...args));
    hook.beforeEach((...args) => this.beforeEach(...args));
    hook.afterEach((...args) => await this.afterEach(...args));
  }
  async onInit() {
    this.myVar = 1;
  }
  async onBeforeEach(content) {
    // Do something with this.myVar...
  }
  async onAfterEach(html) {
    // Do something with this.myVar...
  }
};

class SomePlugin2 extends SomePlugin {
  onInit() {
    super.onInit()
    this.otherVar = 1
  }
  onBeforeEach(content) {
    const md = super.onBeforeEach(content)
    // Do something with this.otherVar...
  },
  async onAfterEach(html) {
    const html = await super.onAfterEach(html)
    // Do something with this.otherVar...
  }
}

[jhildenbiddle]

Respectfully, I think you'll have a hard time convincing people that "classes are easier" in JavaScript in 2020. That argument may have gotten some traction in 2015/2016 when ES6 classes were introduced or even today with new-to-JS devs coming from a class-based OOP language like C++ or Java, but there is a growing consensus in the JS community that the "syntactic sugar" of ES6 classes should (usually) be avoided.

I could reference dozens of articles that explain why JavaScript classes should be avoided (1, 2, 3, 4, 5, 6 ...), but I think the React team's move away from JS classes serves as a good testimonial, as does the Classes confuse both people and machines section of the official React docs:

Classes confuse both people and machines

In addition to making code reuse and code organization more difficult, we’ve found that classes can be a large barrier to learning React. You have to understand how this works in JavaScript, which is very different from how it works in most languages. You have to remember to bind the event handlers. Without unstable syntax proposals, the code is very verbose. People can understand props, state, and top-down data flow perfectly well but still struggle with classes. [...]

Additionally, React has been out for about five years, and we want to make sure it stays relevant in the next five years. As Svelte, Angular, Glimmer, and others show, ahead-of-time compilation of components has a lot of future potential. Especially if it’s not limited to templates. Recently, we’ve been experimenting with component folding using Prepack, and we’ve seen promising early results. However, we found that class components can encourage unintentional patterns that make these optimizations fall back to a slower path. Classes present issues for today’s tools, too. For example, classes don’t minify very well, and they make hot reloading flaky and unreliable. We want to present an API that makes it more likely for code to stay on the optimizable path.

This particular line should not be overlooked:

Without unstable syntax proposals, the code is very verbose

The class examples you provided use public and private class fields, neither of which are supported by all "evergreen" browsers today. So, those example would either need to be modified to use the more verbose and error-prone class syntax from ES2015 only, or they would need to be transpiled to ES2015 using babel or a similar tool. I would hate to see transpilation become a requirement for writing docsify plugins, especially given how simple and convenient they are for end users to just add to their index.html today.

The other articles hit on many of the same points as well as some others. They are worth reviewing if the idea of avoiding classes in JavaScript is new or seems controversial.

So, back to the examples...

I think using this is as simple

JavaScript's this keyword is inarguably one of the most difficult concepts for JS devs to wrap their head around, primarily because its meaning changes based on its context. Let's look at the multiple-method example your provided:

class SomePlugin {
  onInit() {
    // How should I access this?
    this.myVar = 1;
  }

  onBeforeEach(content) {
    const myArray = [1,2,3];

    const myNewArray1 = myArray.map(function(item) {
      // This will throw an error (`this` context has changed in function)
      return item + this.myVar;
    });

    // Function Fix #1: Store a reference to `this.myVar` before using it in
    // in a function declaration. Works, but this requires additional code
    // and it needs to be done for every variable in every method.
    const myVarRef = this.myVar;
    const myNewArray2 = myArray.map(function(item) {
      return item + myVarRef;
    });

    // Function Fix #2: Use arrow functions! Okay, but now we're requiring
    // devs to write code a specific way just do something fairly simple like
    // access a shared variable.
    const myNewArray3 = myArray.map(item => item + this.myVar);

    // What about objects?
    const myObj1 = {
      // This will throw an error (`this` context has changed in object)
      prop1: this.myVar + 1
    };

    // Object Fix #1: Reference `this` outside of the object
    const myObj2 = {};
    myObj2.prop1 = this.myVar + 1

    // Object Fix #2: Or use the same `this.myVar` reference
    // created above (again, works but requires more code)
    const myObj3 = {
      prop1: this.myVarRef + 1
    };
  }
};

The same issue exists for shared functions:

class SomePlugin {
  // How should I access this?
  mySharedFn(str) {
    console.log(str);
  }

  onBeforeEach(content) {
    function myPrivateFn(str) {
      this.mySharedFn(str);
    }

    // Works
    this.mySharedFn(str);

    // This will throw an error (`this` context has changed in myPrivateFn)
    myPrivateFn(str);

    ['foo', 'bar', 'baz'].forEach(function(item) {
      // This will throw an error (`this` context has changed in function)
      this.mySharedFn(item);
    });
  }
}

Now compare that to using a single method (called definition to match pervious examples, but the name could be plugin, pluginFn`, ... whatever):

{
  // ...
  definition(hook) {
    const myVar = 1;

    hook.beforeEach(content => {
      const myArray = [1,2,3];

      function mySharedFn(str) {
        console.log(str);
      }

      // Works
      mySharedFn('Works!');

      const myNewArray1 = myArray.map(function(item) {
        // Works
        return item + myVar;

        // Works
        mySharedFn('Works!')
      });

      // Works
      const myNewArray2 = myArray.map(item => item + myVar);

      const myObj1 = {
        // Works
        prop1: myVar + 1
        myFn() {
          // Works
          mySharedFn('Works!')
        }
      };
    });
  }
};

This also makes migrating v4 plugins to v5 extremely simple: copy v4 plugin logic, update as needed for v5, paste new logic into object as key:function (or use method definition shorthand) along with a few other props:

// v4 Plugin
function(hook, vm) {
  // Do stuff (v4)
}

// v5 Plugin
{
  pluginVersion: "5",
  docsifyVersion: "5.0.0",
  plugin(hook) {
    // Do stuff (v4 => v5)
  }
}

So, which is easier again? 😉

Using this, the object can be more easily extended, and we can use super to call parent methods, and read parent state, etc, which I think is a nice benefit

This sounds like "classes are better at behaving like classes". When would a docsify plugin author leverage these things? How is forcing a class-based plugin system better than, say, using factory functions?

For comparison, here's the equivalent code to achieve variable sharing and extensibility if we go the single-function approach

Your example assumes that we need to use classes. I'm proposing we don't need nor do we want to.

[trusktr]

Respectfully

No worries, nothing wrong with expressing opinions, in my opinion. :)

I've read those arguments against classes before. I still believe they are great for certain situations and not for others. It's not an all-or-nothing type of thing.

Even with object literals, someone can still introduce the problem by placing a method on the object literal, and passing the method somewhere else.

const myPlugin = {
  someArray: [ ... ],
  someMethod(item) { ... },
  definition(hook) {
    this.someArray.map(this.someMethod) // bad
  },
}

this is something people need to learn to be effective with JavaScript.

---

Side note, not sure what you meant about

    // What about objects?
    var myObj1 = {
      // This will throw an error because the meaning of `this` changes when
      // called within an object.
      prop1: this.myVar + 1
    };

The following works with no error:

class Foo {
  myVar = 123

  test() {
    var myObj1 = {
      prop1: this.myVar + 1 // no error
    };
    console.log(myObj1.prop1 === this.myVar + 1) // logs "true"
  }
}

const f = new Foo
f.test() // logs "true"

Regarding the following,

    ['foo', 'bar', 'baz'].forEach(function(item) {
      // ... but this throw an error because the meaning of `this`
      // changes when called within a function.
      this.mySharedFn(item);
    });

it is a "novice" mistake that with a little experience can be avoided, and it isn't a problem unique to classes.

---

Note that with the current proposal of the API accepting objects, we can't prevent people from using class.

If we were to go with the definition() method approach, I could write myself a base class to get the functionality I'd like, as follows:

class DocsifyPlugin {
  definition(hook) {
    hook.init(async (...args) => this.init && await this.init(...args));
    hook.beforeEach(async (...args) => this.beforeEach && await this.beforeEach(...args));
    hook.afterEach(async (...args) => this.afterEach && await this.afterEach(...args));
    // ... etc ...
  }
}

class CoolPlugin extends DocsifyPlugin {
  version = "4"
  docsifyVersion = "4.11.0"
  async init() { /* ... */ }
  async beforeEach(md) { /* ... */ }
  async afterEach(html) { /* ... */ }
  // ... etc ...
}

classes are better at doing the things classes do

Yes, and the things classes do, some people find useful. :) It may be a matter of taste and opinion in many cases, and not a matter of fact.

When would a docsify plugin author leverage these things?

We can't exactly answer the question of when an author would want to leverage classes specifically: we don't know what plugins people are going to make and for what purpose, only that we want our system to be extensible enough for people to have creative freedom.

But in the general sense, if someone writes some piece of functionality, it is conceivable they may want to share that logic between similar or related plugins.

What we can effectively debate about is how we'll make the system extensible, and in which ways we allow authors to extends it, but it is difficult to imagine if they'll need specifically classes or not.

How is forcing a class-based plugin system better than, say, using factory functions?

What I proposed isn't forcing a class-based plugin system (plus forcing it wouldn't necessarily be better).

For example, there are no classes in the following:

window.$docsify = {
  plugins: [
    {
      version: "4",
      docsifyVersion: "4.11.0",
      beforeEach(content) { ... },
      afterEach(content) { ... },
    }
  ]
}

To satisfy the use case of having function-scope variables shared among hooks, here it is with a factory function as well as a clean list of hook methods at the same time:

const makeSomePlugin() {
  const myVar = 123

  return {
    version: "4",
    docsifyVersion: "4.11.0",
    init() { /* ... uses myVar ... */ },
    beforeEach(content) { /* ... uses myVar ... */ },
    afterEach(content) { /* ... uses myVar ... */ },
  }
}

window.$docsify = {
  plugins: [ makeSomePlugin() ]
}

^ That is cleaner than passing callbacks into hook functions, in my opinion. Wdyt?

Likewise, someone could also choose to make a special type of "factory function" that must be called with new, if they wish to take advantage of certain language features:

 class OtherPlugin extends SharedLogic { ... }
 class AnotherPlugin extends SharedLogic { ... }

window.$docsify = {
  plugins: [ makeSomePlugin(), new OtherPlugin(), new AnotherPlugin() ]
}

Both the class and factory patterns are optional tools, neither being specifically required.

So, which is easier again? 😉

I like the list of methods inside the object because it is neater to write than passing callbacks into the hook functions, and if someone wants to encapsulate variables they can do so with a factory function if they desire, and someone else can use a class if they wish, and another person can use a plain object literal if they like.

Plus passing callbacks into the hook functions can potentially introduce the context-switching problem anyway if someone does it the wrong way. It's really just a problem in general that everyone should learn about if they wish to code in JS properly. I don't think there is any way around that.

[jhildenbiddle]

No worries, nothing wrong with expressing opinions, in my opinion. :)

Agreed. Didn't mean the "respectfully" to sound snarky.

As for the other comments, I think there is some confusion about where I'm discussing the use of JavaScript classes as opposed to single vs. multiple methods. For example, my comments on the pitfalls of relying on this are related to the single vs multiple method debate, not classes (since the same issues with this exist with both classes and objects). Class-related comments are intended to demonstrate how requiring docsify plugin authors to adopt them would be (IMHO) a mistake, not a criticism of those who find them useful. If JavaScript classes butter your biscuit, grab some biscuits and have at it.

I've read those arguments against classes before. I still believe they are great for certain situations and not for others. It's not an all-or-nothing type of thing.

Sure. People have preferences and things can usually be done multiple ways. The discussion changes when preferences have repercussions that impact others, as is the case with requiring others to use JavaScript classes. See: React.

What I proposed isn't forcing a class-based plugin system

Perfect. I incorrectly assumed it was when you spoke of extending classes, calling parent methods, and reading parent state (i.e. having plugins extend from a docsify parent class of some sort, similar to React.Component).

this is something people need to learn to be effective with JavaScript.
it is a "novice" mistake that with a little experience can be avoided

And yet it remain an issue after 25 years of JavaScript. You may get it, but I don't think assuming others do (and if they don't, they should) is a good way to approach designing software for a wide range of people. We should be removing stumbling blocks, not introducing them or leaving them in place because we prefer organizing our code a certain way.

const makeSomePlugin = () => {
  const myVar = 123

  return {
    version: "4",
    docsifyVersion: "4.11.0",
    init() { /* ... uses myVar ... */ },
    beforeEach(content) { /* ... uses myVar ... */ },
    afterEach(content) { /* ... uses myVar ... */ },
  }
}

window.$docsify = {
  plugins: [ makeSomePlugin() ]
}

^ That is cleaner than passing callbacks into hook functions, in my opinion. Wdyt?

👍 Yep (fixed method shorthand syntax usage to fat arrow function).

Someone could also choose to make a special type of "factory function" that must be called with new

They could. I hope they wouldn't to avoid the inevitable bugs from this:

window.$docsify = {
  plugins: [ 
    myPlugin1,      // object
    myPlugin2(),    // function
    new myPlugin3() // class
  ]
}

Knowing that the recommendations we make and tools we provide will heavily influence how people write docsify-related plugin code, my hope would be that we would favor something like this:

window.$docsify = {
  plugins: [ 
    myPlugin1(), 
    myPlugin2(), 
    myPlugin3() 
  ]
}