Wrapper function for creating plugins #1209
Comments
|
@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 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() {});
// ...
}
});
This is just an example I am thinking of the implementation. We can discuss this for a better approach. From #1061 (comment) |
|
Good stuff, @anikethsaha. The Regarding the wrapper function, if we move from 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 |
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 🔼
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.
yes |
|
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:
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 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(),
// ...
]
}; |
|
This makes me think: what if instead of passing 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(),
// ...
]
}; |
One advantage of passing {
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 {
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 |
I think using {
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 Using 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...
}
} |
|
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:
This particular line should not be overlooked:
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 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...
JavaScript's 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(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? 😉
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?
Your example assumes that we need to use classes. I'm proposing we don't need nor do we want to. |
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
},
}
Side note, not sure what you meant about
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,
it is a "novice" mistake that with a little experience can be avoided, and it isn't a problem unique to Note that with the current proposal of the API accepting objects, we can't prevent people from using If we were to go with the 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 ...
}
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.
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.
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 class OtherPlugin extends SharedLogic { ... }
class AnotherPlugin extends SharedLogic { ... }
window.$docsify = {
plugins: [ makeSomePlugin(), new OtherPlugin(), new AnotherPlugin() ]
}Both the
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 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. |
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
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.
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
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() ]
}
👍 Yep (fixed method shorthand syntax usage to fat arrow function).
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()
]
} |
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:
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).
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:
try { ... } catch(err) { ...}statement to prevent a plugin error from breaking a site.$doscifyobject ($docsify.tabsand$docsify.themeable) with avaluekey 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.Just some ideas to get started.
Originally posted by @jhildenbiddle in #1061 (comment)
The text was updated successfully, but these errors were encountered: