Add plugins functionality

[jamos-tay]

What is the purpose of this pull request? (put "X" next to an item, remove the rest)

• [X] New feature

Fixes #470

What is the rationale for this request?

Allow MarkBind to use plugins, making it more customisable.

What changes did you make? (Give an overview)

Add plugins functionality:
Plugins are placed in _markbind/plugins folder.
User specifies plugins to use in site.json.
Plugins allow user to specify a preRender and postRender function.

// myPlugin.js

const cheerio = module.parent.require('cheerio');

module.exports = {
  postRender: (content, siteContext) => {
    const $ = cheerio.load(content, { xmlMode: false });
    // Modify the page...
    $('#my-div').append(siteContext["post"]);
    return $.html();
  },
};

// site.json

{
  ...
  "plugins" : [
    "myPlugin"
  ],
  "context" : {
    "plugin1" : {
      "post" : "<p>Hello</p>"
    }
  }
}

// index.md

...
<div id="my-div"></div>

Is there anything you'd like reviewers to focus on?

How do we want to handle plugin dependencies (require)? Right now I'm just using module.parent.require to get libraries from the markbind source, but if the user wants to use his own libraries he'd have to install them somewhere in his project. Perhaps within the plugin folder?

[yamgent]

Bare in mind that authors will not have direct access to the source, as they install MarkBind with npm install -g markbind-cli. Thus, a fully fledged plugin system will need to be able to extend MarkBind's functionalities without having to touch MarkBind's source code.

[jamos-tay]

Sure - I don't think that's a problem.

[jamos-tay]

Updated

[jamos-tay]

Resolved merge conflict and updated, sorry I'm not sure how so many of them got missed out,...

[acjh]

Does this mean that we expect user to copy-paste the file if #467 is converted into a plugin?

1. Ideally, plugins can be installed via NPM. This allows plugins to be shared, like ESLint.
2. There can be built-in plugins for convenience, which will likely form the bulk of MarkBind plugins. These could be from internal development (e.g. Add element tagging functionality #467) or accepting community contributions.
3. Supporting local plugins (which ESLint does not) in _markbind/plugins/ is a bonus, but this is considered advanced usage.

When looking for plugins, we should use the order 3 → 1 → 2 to facilitate overrides.

[jamos-tay]

Does this mean that we expect user to copy-paste the file if #467 is converted into a plugin?

I was thinking we can have a few plugins that are 'installed' by default that the user can enable or disable, so they won't have to download separate files.

1. Ideally, plugins can be installed via NPM. This allows plugins to be shared, like ESLint.

Would this be installed in the markbind source folder? Like @yamgent mentioned we probably don't want them touching the source. Either that or they can install them in to the project folder, but that's still a little troublesome, and it also means they can't be shared among projects.

What do you think of a markbind install command specifically for installing plugins? (This is long term though so we probably won't need it any time soon)

2. There can be built-in plugins for convenience, which will likely form the bulk of MarkBind plugins. These could be from internal development (e.g. Add element tagging functionality #467) or accepting community contributions.

We can have a plugins folder in the source that is installed with a few preset plugins

[acjh]

I was thinking we can have a few plugins that are 'installed' by default that the user can enable or disable, so they won't have to download separate files.

Do you mean built-in plugins? This MR does not support that yet right?

Would this be installed in the markbind source folder?

No, it's installed into the project directory, like ESLint.

it also means they can't be shared among projects.

Can you elaborate why they can't be shared?

What do you think of a markbind install command specifically for installing plugins?

No, we can use NPM.

We can have a plugins folder in the source that is installed with a few preset plugins

Yes, that was the original intention — built-in plugins.

[jamos-tay]

By project directory are you referring to the markbind-cli directory? If that's the case sharing is okay, I thought it meant the folder for a specific markbind website (like book) in which case they can't be accessed by other websites

[acjh]

By project directory are you referring to the markbind-cli directory?

No, I'm referring to the project directory — the folder for a specific markbind website (like book).

This MR uses the project directory to support local plugins, but the user has to copy-paste the file.

NPM modules can be installed by multiple projects, which is what I mean by shared.

[jamos-tay]

Ah I see, but the user still has to run npm install on each individual project, right?

[acjh]

Yes, that's how ESLint plugins are installed.

[damithc]

In the first stage of adding this feature, we could limit it to simply a way for the dev team to add functionality easily i.e., the plugin mechanism is not necessarily visible to end users. As our philosophy is to optimize for a specific target user group, we want most functionality to be in the box (they can be plugins enabled by default) and ready-to-use by that target user group, which means only in a rare case a user will need to tinker with plugins.

[jamos-tay]

@damithc Ah - In that case should the options in the site.json be removed?

[acjh]

No, users should be able to remove plugins.

[damithc]

No, users should be able to remove plugins.

👍

[acjh]

We still need to support built-in plugins.

[jamos-tay]

Updated, rebased on master to solve merge conflict

[jamos-tay]

Updated, renamed siteContext to pluginsContext

[acjh]

We still need to support built-in plugins.

[jamos-tay]

@acjh Hi, just to confirm, what other changes do you need from this PR?

For the built-in (installed by default) plugins I was thinking of adding them in the PR where we actually start converting the features into plugins, if I wrote the feature at this point the functionality would likely be placeholder and would probably have to be rewritten in future.

[acjh]

Hmm, but this PR only supports the usage with the lowest priority.

Does this mean that we expect user to copy-paste the file if #467 is converted into a plugin?

1. Ideally, plugins can be installed via NPM. This allows plugins to be shared, like ESLint.
2. There can be built-in plugins for convenience, which will likely form the bulk of MarkBind plugins. These could be from internal development (e.g. Add element tagging functionality #467) or accepting community contributions.
3. Supporting local plugins (which ESLint does not) in _markbind/plugins/ is a bonus, but this is considered advanced usage.

When looking for plugins, we should use the order 3 → 1 → 2 to facilitate overrides.

[jamos-tay]

Updated, I'm getting a problem with netlify, can someone help me with this error?

Edit: resolved

[yamgent]

Resolve conflicts.

[jamos-tay]

Rebased!

[damithc]

@jamos-tay can users do anything with the built-in plugins? e.g., disable them

[jamos-tay]

Oh they are disabled by default, to enable them the user has to put the plugin name in plugins under the site.json

[damithc]

Oh they are disabled by default, to enable them the user has to put the plugin name in plugins under the site.json

So the tags feature no longer works by default?

[jamos-tay]

Oh yeah, this PR changes the way the site json is specified:

Old:

{
...
tags: [...]
}

New:

{
  ...
  "plugins" : [
    "filterTags"
  ],
  "pluginsContext" : {
    "filterTags" : {
      "tags": [...]
    }
  }
}

The reason is because we didn't want to clutter up the top level of the configuration with less important properties. I think the section in the docs wasn't cleaned up, let me do it now.

[damithc]

Specifically, if I want to use tags in a page via frontmatter, (i.e., not via site.json), how do I do that?

[jamos-tay]

The .md file doesn't change, pass in an empty array for the context:

  "pluginsContext" : {
    "filterTags" : {
      "tags": []
    }
  }

Hmm, I think I should make it such that it defaults to empty because not specifying an empty array causes errors

[damithc]

A few other thoughts:

• Can we use pluginsParameters instead of pluginContexts? The former sounds less scary to those with minimal programming knowledge (i.e., our target users)
• Can we specify plugin and the parameters together, rather than in two places?

[jamos-tay]

You mean like

{
  ...
  "plugins" : [
    "filterTags" : {
      "tags": [...]
    }
  ]
}

?
Make sense, I'm not sure why we didn't do that to begin with actually.

[acjh]

The benefits of specifying it separately:

• ability to toggle on/off without removing the parameters entirely.
• ability to imply defaults without passing null or {}, which may appear to be forcing the user to set something.

[jamos-tay]

Should it be kept the way it is then? There are a number of plugins that don't require parameters (anchors, syntax improvements) so empty defaults may look ugly.

[damithc]

Should it be kept the way it is then? There are a number of plugins that don't require parameters (anchors, syntax improvements) so empty defaults may look ugly.

Sure.

BTW, I'm still inclined to enable built-ins by default and allow disabling if they want to e.g., for performance reasons. As our promise is to optimize for a certain target user group, it is our job to provide all the needed functionality out of the box. It can also be one way we differ from others. i.e., 'All features built-in; no need to fiddle with plugins'

[acjh]

It was also mentioned by @jamos-tay in #495 (comment).

We can allow disabling by accepting "off" in "pluginsContext", similar to ESLint?
https://github.com/MarkBind/markbind/blob/master/.eslintrc.js