[PoC]Plugin architecture using webpack (HTML \class extension) by ylemkimon · Pull Request #1074 · KaTeX/KaTeX

ylemkimon · 2018-01-18T19:01:39Z

Depends on #1068
Fixes #762
Fixes #90, fixes #507

See 5824510.

This is a proof of concept of implementing a plugin architecture using webpack and implements non-standard \class{name}{math} extension. Using CommonsChunkPlugin, it is possible to build codes using KaTeX internal functions, without exposing them to the public API(although it exposes webpackJsonp__name_ to the global) or duplicating their codes.

Things to consider:

Custom lexer/parser
Plugin registry
3rd party plugin
Backward/Forward compatibility

Use NamedModulesPlugin and HashedModuleIdsPlugin
Version range check
Used features list
Runtime tests
Wrap codes in try-catch, and unregister the plugin if error is caught

khanbot · 2018-01-18T19:01:48Z

CLA signature looks good 👍

ylemkimon · 2018-01-18T19:49:26Z

static/index.html

        <title>KaTeX Test</title>
        <script src="katex.js" type="text/javascript"></script>
-        <link href="katex.css" rel="stylesheet" type="text/css">
+        <script src="contrib/html.js" type="text/javascript"></script>


The user can use the plugin by loading after the KaTeX.

The file looks like:

(function webpackUniversalModuleDefinition(root, factory) { if(typeof exports === 'object' && typeof module === 'object') module.exports = factory(); else if(typeof define === 'function' && define.amd) define([], factory); else if(typeof exports === 'object') exports["contrib/html"] = factory(); else root["contrib/html"] = factory(); })(this, function() { return webpackJsonp_name_([0],{ /***/ 59: /***/ (function(module, __webpack_exports__, __webpack_require__) { "use strict"; Object.defineProperty(__webpack_exports__, "__esModule", { value: true }); /* harmony import */ var __WEBPACK_IMPORTED_MODULE_0_babel_runtime_helpers_toConsumableArray__ = __webpack_require__(30); /* harmony import */ var __WEBPACK_IMPORTED_MODULE_0_babel_runtime_helpers_toConsumableArray___default = __webpack_require__.n(__WEBPACK_IMPORTED_MODULE_0_babel_runtime_helpers_toConsumableArray__); /* harmony import */ var __WEBPACK_IMPORTED_MODULE_1__src_defineFunction__ = __webpack_require__(2); /* harmony import */ var __WEBPACK_IMPORTED_MODULE_2__src_buildCommon__ = __webpack_require__(0); /* harmony import */ var __WEBPACK_IMPORTED_MODULE_3__src_mathMLTree__ = __webpack_require__(1); /* harmony import */ var __WEBPACK_IMPORTED_MODULE_4__src_domTree__ = __webpack_require__(12); /* harmony import */ var __WEBPACK_IMPORTED_MODULE_5__src_buildHTML__ = __webpack_require__(3); /* harmony import */ var __WEBPACK_IMPORTED_MODULE_6__src_buildMathML__ = __webpack_require__(4); Object(__WEBPACK_IMPORTED_MODULE_1__src_defineFunction__["b" /* default */])({ type: "class", names: ["\\class"], props: { numArgs: 2, allowedInText: true, greediness: 3, argTypes: ["string", "original"] }, handler: function handler(context, args) { var classes = args[0]; var body = args[1]; return { type: "class", classes: classes.value, value: Object(__WEBPACK_IMPORTED_MODULE_1__src_defineFunction__["c" /* ordargument */])(body) }; }, htmlBuilder: function htmlBuilder(group, options) { var elements = Object(__WEBPACK_IMPORTED_MODULE_5__src_buildHTML__["a" /* buildExpression */])(group.value.value, options, false); var classes = group.value.classes.trim().split(/\s+/); var fragment = new __WEBPACK_IMPORTED_MODULE_2__src_buildCommon__["a" /* default */].makeFragment(elements); fragment.children.forEach(function (children) { if (!(children instanceof __WEBPACK_IMPORTED_MODULE_4__src_domTree__["a" /* default */].svgNode)) { var _children$classes; (_children$classes = children.classes).push.apply(_children$classes, __WEBPACK_IMPORTED_MODULE_0_babel_runtime_helpers_toConsumableArray___default()(classes)); } }); return fragment; }, mathmlBuilder: function mathmlBuilder(group, options) { var inner = Object(__WEBPACK_IMPORTED_MODULE_6__src_buildMathML__["a" /* buildExpression */])(group.value.value, options); var node = new __WEBPACK_IMPORTED_MODULE_3__src_mathMLTree__["a" /* default */].MathNode("mstyle", inner); node.setAttribute("class", group.value.classes); return node; } }); /***/ }) },[59])["default"]; });

ry-randall · 2018-01-19T14:44:18Z

This is really exciting to see. I'd like to take a deeper look at this soon. Would applying the non-standard \id{name}{latex} follow a similar pattern?

ry-randall · 2018-01-19T14:46:23Z

Makefile

@@ -1,5 +1,5 @@
-.PHONY: build dist lint setup copy serve clean metrics test coverage zip contrib flow
-build: test build/katex.min.js build/katex.min.css contrib zip compress
+.PHONY: build dist lint setup webpack serve clean metrics test coverage zip flow


Assuming most of the infrastructure related changes go away once the webpack PR is merged?

What changes are specific to this PR?

@rrandallcainc Yes. Currently only the last commit(5824510) is specific to this PR.

Doh, should have read your full comment :)

ylemkimon · 2018-01-19T15:00:08Z

@rrandallcainc As an element ID should be unique, I think it should be applied to the enclosing span.

ry-randall · 2018-01-19T15:21:38Z

Just want to make sure I understand this plugin system:

Since there are two separate entry points (contrib/html, and katex) which both share common KaTeX code, you use the CommonsChunkPlugin to extract the KaTeX specific code into it's own chunk, which these two entry points can share. This ensures that there's no code duplication, and contrib/html can leverage common katex code. Is that correct?

Might need clarification from @kevinbarabash but was the intention to inject plugin functionality at run-time, not build-time? In other words, I think this change is dependent on the webpack config. If an outside user wants to plugin their own custom function, would they need to create a webpack config to allow them to do so?

ry-randall · 2018-01-19T16:10:56Z

contrib/html/html.js

+import {buildExpression as htmlBuildExpression} from "../../src/buildHTML";
+import {buildExpression as mmlBuildExpression} from "../../src/buildMathML";
+
+defineFunction({


It feels a little odd that some of this logic lies outside of the source (like this), but other logic needs to lie within the source (like Parser/types). That being said, I'm not sure there's a better way to implement it at this time without a major change.

ylemkimon · 2018-01-19T16:38:10Z

@rrandallcainc

Since there are two separate entry points (contrib/html, and katex) which both share common KaTeX code, you use the CommonsChunkPlugin to extract the KaTeX specific code into its own chunk, which these two entry points can share.

The common codes(KaTeX specific codes) are extracted to the katex module, where they were originally, so KaTeX codes are left in the katex module.

This ensures that there's no code duplication, and contrib/html can leverage common katex code. Is that correct?

Yes.

In other words, I think this change is dependent on the webpack config. If an outside user wants to plugin their own custom function, would they need to create a webpack config to allow them to do so?

Yes, the developer has to have KaTeX sources and add an entry to the webpack config to build a plugin. It will be like a SDK and maybe we can provide a boilerplate.

It feels a little odd that some of this logic lies outside of the source (like this), but other logic needs to lie within the source (like Parser/types). That being said, I'm not sure there's a better way to implement it at this time without a major change.

I'm thinking of adding defineGroupParser function, which add custom group parsers to the Parser's parseGroupOfType like below.

I think with defineGroupParser, defineFunction, defineMacro, defineEnvironment, most of features can be implemented without changing KaTeX's sources.

ylemkimon · 2018-01-19T16:49:31Z

src/Parser.js

+type GroupParser = (optional: boolean) => ?ParsedFuncOrArgOrDollar;
+const GROUP_PARSER_EXTENSIONS: { [type: string]: GroupParser } = {};
+export function defineGroupParser(type: string, parser: GroupParser) {
+    GROUP_PARSER_EXTENSIONS[type] = parser;


Interesting idea. This could also be used for defining a custom parser for \ce from the mhchem package.

k4b7 · 2018-01-19T17:52:57Z

My original thinking of a plugin system is that it would be more of a runtime system thing. I hadn't considered leveraging the fact that we're moving to webpack to build KaTeX. It feels a little weird imposing plugin makers use a particular build system. I need to think about it a bit more.

A runtime plugin system would require us to expose more methods for generating HTML and MathML. I think if/when we moved to HAST for our internal representation of HTML and MathML then we wouldn't have to expose as much. If/when we move to an intermediate representation we'd still want to expose methods for building that IR.

k4b7 · 2018-01-22T04:46:23Z

@ylemkimon can you rebase when you have a chance? I'm curious to see what this diff looks like now that the webpack changes have been merged.

ylemkimon · 2018-01-22T05:06:27Z

@kevinbarabash Done.

k4b7

I think this is probably okay for building extensions in the contrib folder. We still have a long way to go in cleaning up and stabilizing internal data structures before we'll have something that can be used for runtime plugins. I like that \class is being defined in the contrib folder as it's not standard LaTeX.

k4b7 · 2018-01-22T05:20:51Z

contrib/html/html.js

+    return newArgument(new ParseNode("string", res.text, "text"), res);
+});
+
+// $FlowFixMe


What's the error?

@kevinbarabash props.argTypes expects an array of ArgType, so it doesn't recognize custom string group type. I couldn't find a way to extend ArgType from defineGroupParser.

I think it's fine to just change it in types.js. Eventually though, we may say that

type ArgType = string;

and add other programmatic checks as necessary.

k4b7 · 2018-01-22T05:21:41Z

src/Parser.js

+type GroupParser = (optional: boolean) => ?ParsedFuncOrArgOrDollar;
+const GROUP_PARSER_EXTENSIONS: { [type: string]: GroupParser } = {};
+export function defineGroupParser(type: string, parser: GroupParser) {
+    GROUP_PARSER_EXTENSIONS[type] = parser;


Interesting idea. This could also be used for defining a custom parser for \ce from the mhchem package.

k4b7 · 2018-01-22T05:23:22Z

src/Parser.js

@@ -758,6 +764,12 @@ export default class Parser {
            return this.parseUrlGroup(optional);
        }


Maybe defineGroupParser can be used for color, size, url, and even original.

k4b7 · 2018-01-22T05:26:02Z

contrib/html/html.js

+            if (!(children instanceof domTree.svgNode)) {
+                children.classes.push(...classes);
+            }
+        });


I would've expected all of the elements to be wrapped in a single span with the given class applied. This applies the class to each element. Is that what MathJax does?

@kevinbarabash No, MathJax does what you said:

all of the elements to be wrapped in a single span with the given class applied.

Though, I'm not sure which is more suitable. For wrapping in a span, I think we can do in a similar way to \href:
https://github.com/Khan/KaTeX/blob/2a2742453b86fba6fd5184efb3c3255400796fb7/src/functions/href.js#L25-L66

Blocked on #1070

#1070 has been merged, but I think we should probably do what MathJax does since that's probably the only other TeX renderer which supports \class.

k4b7 · 2018-01-22T05:27:20Z

contrib/html/html.js

+        const inner = mmlBuildExpression(group.value.value, options);
+        const node = new mathMLTree.MathNode("mstyle", inner);
+        node.setAttribute("class", group.value.classes);
+        return node;


The behavior of the MathML builder is quite different from the HTML builder. They should probably have similar semantics, e.g. apply the class to each individual element or wrapping all of the in a parent element.

k4b7 · 2018-01-22T05:29:17Z

webpack.common.js

 type Target = {|
    name: string, // the name of output JS/CSS
-    entry: string, // the path to the entry point
+    entry: string | Object, // the path to the entry point


It's too bad there aren't any existing flow type definitions for webpack.

@kevinbarabash There seems to be an open issue in flow-typed/flow-typed#165. Also webpack has JSON Schema(schemas/webpackOptionsSchema.json) and maybe we can convert it using json-schema-to-flow-type. I wonder there is any way to import JSON schema to flow directly.

I wouldn't worry about it for this PR.

k4b7 · 2018-01-22T05:32:09Z

webpack.common.js

+            target.name === 'katex' && new webpack.optimize.CommonsChunkPlugin({
+                name: 'katex',
+                minChunks: Infinity,
+            }),


So if any modules from the katex chunk also appear in any other chunk (e.g. the contrib/html) remove them from the other chunk.

@kevinbarabash Yes 😄

ry-randall · 2018-01-23T14:31:38Z

I think if we go with this way for now, it would be good to add some documentation around how users can leverage this plugin system.

ylemkimon · 2018-01-23T15:52:29Z

@rrandallcainc I think we can provide a boilerplate plugin.

k4b7 · 2018-01-25T05:34:30Z

Custom lexer/parser

Looks like you have at least the parser half done.

Plugin registry

We need some plugins first. :P

3rd party plugin

I suppose that someone could make one use the technique demonstrated in this PR. The problem though is that they'd need to require a number of files directly from the source. Pretty much every JS library I've seen that supports plugins allows to create a plugin either without an dependencies or by requiring some utilities to help work with the data structures the library uses.

I think the code in this PR is useful. It expands the number of extension points we have by introducing defineGroupParser. It also provides a way for contrib extensions to access internals without duplicating code that already exists in the katex bundle. I definitely want to merge this PR, but I would hesitate to call it a plugin architecture, mainly b/c I think people have certain expectations of JS plugin systems.

This could use some documentation. Could you add an Extending KaTeX section to README.md and describe when you would want to create an extension using this technique and what are the main steps in doing so?

Backward/Forward compatibility

We'd probably want some sort of versioning system. Even before that though there are some key changes that I'd like to make first:

simplify our parse tree (flatten and make it more regular?)
swap out domTree for HAST
introduce and intermediate representation

I think the last one may take a while so maybe that's a v2 thing and maybe we can move KaTeX to v1 after we finish the other two.

k4b7

Requesting two changes:

wrap contents of \class in a single span
update types.js and remove $FlowFixMe

ry-randall · 2018-02-01T14:33:46Z

Agreed with \class being a single span. I assume that it would be easy to have \id follow the same pattern?

ylemkimon · 2018-02-01T15:20:51Z

@rrandallcainc Yes, if we use single span, we can just set the id of it.

Blocked on #1103.

k4b7 · 2018-02-03T06:06:39Z

@ylemkimon I wonder if you had any thoughts about how we might allow plugins/extensions to add fonts and font metrics. I looking into generating Cyrillic fonts. If we add support for more scripts with fonts and metrics it would be nice to allow people to load only those which they think they might need.

k4b7 · 2018-02-20T02:27:51Z

I believe this is no longer blocked by other PRs.

ylemkimon · 2018-02-27T18:48:01Z

@kevinbarabash Sorry, I was on a trip 😄 This is still blocked by spacing issues like #1108 and #1154.

dvergeylen · 2019-02-05T10:18:56Z

Hi,

May I ask why this PR has been closed? Had it been included in other commits of some kind?

I would be very interested by adding custom classes.

#1108 and #1154 are both closed by now, is there anything else/reason blocking this PR?

k4b7 · 2019-03-17T22:48:20Z

Custom classes (and ids) are being added in #1437.

ylemkimon commented Jan 18, 2018

View reviewed changes

ylemkimon changed the title ~~[PoC]Plugin architecture using webpack~~ [PoC]Plugin architecture using webpack (HTML \class extension) Jan 19, 2018

ry-randall reviewed Jan 19, 2018

View reviewed changes

ylemkimon commented Jan 19, 2018

View reviewed changes

\class plugin demo

e998087

k4b7 reviewed Jan 22, 2018

View reviewed changes

This was referenced Jan 22, 2018

Class #507

Closed

Cursor #506

Closed

Merge branch 'master' into plugin-html

41ac0cd

k4b7 requested changes Feb 1, 2018

View reviewed changes

k4b7 added the GH Review: needs-revision label Feb 1, 2018

ylemkimon mentioned this pull request Feb 1, 2018

Improve JS spacing #1103

Merged

2 tasks

ylemkimon added the blocked label Feb 1, 2018

ronkok mentioned this pull request Feb 4, 2018

mhchem extension support #50

Closed

ylemkimon mentioned this pull request Feb 8, 2018

Supporting Arbitrary Font in Text Mode #1148

Closed

k4b7 removed the blocked label Feb 20, 2018

ylemkimon closed this May 20, 2018

ylemkimon deleted the plugin-html branch May 20, 2018 11:59

ylemkimon restored the plugin-html branch May 20, 2018 11:59

ylemkimon deleted the plugin-html branch July 21, 2018 10:03

		@@ -758,6 +764,12 @@ export default class Parser {
		return this.parseUrlGroup(optional);
		}

Uh oh!

Conversation

ylemkimon commented Jan 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

khanbot commented Jan 18, 2018

Uh oh!

ylemkimon Jan 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ylemkimon Jan 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ry-randall commented Jan 19, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ylemkimon Jan 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ylemkimon commented Jan 19, 2018

Uh oh!

ry-randall commented Jan 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ylemkimon commented Jan 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k4b7 commented Jan 19, 2018

Uh oh!

k4b7 commented Jan 22, 2018

Uh oh!

ylemkimon commented Jan 22, 2018

Uh oh!

k4b7 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ylemkimon Jan 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ylemkimon commented Jan 18, 2018 •

edited

Loading

ylemkimon Jan 18, 2018 •

edited

Loading

ylemkimon Jan 18, 2018 •

edited

Loading

ylemkimon Jan 19, 2018 •

edited

Loading

ry-randall commented Jan 19, 2018 •

edited

Loading

ylemkimon commented Jan 19, 2018 •

edited

Loading

ylemkimon Jan 23, 2018 •

edited

Loading