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

Add plugin hooks #1228

Closed
wants to merge 5 commits into from
Closed

Add plugin hooks #1228

wants to merge 5 commits into from

Conversation

ang-zeyu
Copy link
Contributor

@ang-zeyu ang-zeyu commented May 28, 2020

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

• [x] Other, please explain: plugin scalability (performance)

Resolves #1188 as a first, there's still much room for polish in the plugin api

What is the rationale for this request?

Plugins modify the content currently only through 2 hooks, pre/postRender.
This is very expensive however, as it involves parsing each page an additional time for each plugin there is.

What changes did you make? (Give an overview)

  • Added two new hooks, preRenderNode and postRenderNode. These tap into the same processes as pre/postRender at their respective stages, but directly through the recursive dom handling process.
  • Convert 3 plugins as a start to test the performance improvements ( ~10% ) in three separate commits
    • anchors plugin ⚓
    • plant uml
    • <span heading> shorthand plugin for panels ( converts to <div slot="header"> )

Provide some example code that this change will affect:

Hence instead of reparsing the content for every plugin (cheerio.load), one can write a simple plugin through the following (shorthand plugin)

if (node.name === 'panel' && node.children) {
  node.children.forEach((n) => {
  if (n.name === 'span' && n.attribs.heading !== undefined) {
    n.attribs.slot = 'header';
    n.attribs.class = n.attribs.class ? `${n.attribs.class} card-title` : 'card-title';
    delete n.attribs.heading;
  }
});
return node;

Is there anything you'd like reviewers to focus on?
na, I'm leaving this undocumented for now pending #1151, would also be good if anything needs to be changed in the immediate future

Testing instructions:
npm run test should pass

Proposed commit message: (wrap lines at 72 characters)
Add more plugin hooks

There are only 2 interfaces for plugins to modify a page's content,
preRender and postRender. These plugin hooks do so by passing the
processed page content thus far to the plugin, requiring the plugin to
reparse the entire page content, often only to perform simple
operations.

Let's add 2 more plugin hooks to allow plugins to tap into the
recursive dom handling processes at these respective stages.

In the following commits, we adapt some of the plugins to use these new
hooks, and observe a significant performance improvement of ~10%, which
should likely scale as we add and adapt more plugins.

@ang-zeyu ang-zeyu force-pushed the add-plugin-hooks branch from 96a2aa6 to 6d69e08 Compare May 28, 2020 13:14
ang-zeyu added 5 commits June 10, 2020 18:51
There are only 2 interfaces for plugins to modify a page's content,
preRender and postRender. These plugin hooks do so by passing the
processed page content thus far to the plugin, requiring the plugin to
reparse the entire page content, often only to perform simple
operations.

Let's add 2 more plugin hooks to allow plugins to tap into the
recursive dom handling processes at these respective stages.

In the following commits, we adapt some of the plugins to use these new
hooks, and observe a significant performance improvement of ~10%, which
should likely scale as we add and adapt more plugins.
@ang-zeyu
Copy link
Contributor Author

resolved conflicts and converted the codeBlockCopy plugin as well

@ang-zeyu
Copy link
Contributor Author

implemented in #1403

@ang-zeyu ang-zeyu closed this Dec 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add more hooks for plugins
1 participant