From 9ed4999d1465ad80417f39db108204697bba5ddb Mon Sep 17 00:00:00 2001 From: Jesse Atkinson Date: Wed, 10 Oct 2018 15:27:36 -0700 Subject: [PATCH 1/2] Update tex to include load-mathjax rather than rely on global Summary: Please note that the linter complained about the order of a few methods so I had to move them around. Test Plan: Check out some fixtures that use tex.jsx such as `texified-code.jsx.fixture.js` and ensure everything renders properly. Reviewers: #webpack, john Reviewed By: #webpack, john Subscribers: john, kevinb, davidflanagan Differential Revision: https://phabricator.khanacademy.org/D49193 --- js/tex.jsx | 83 +++++++++++++++++++++++++++++++----------------------- 1 file changed, 48 insertions(+), 35 deletions(-) diff --git a/js/tex.jsx b/js/tex.jsx index d11605f..c7f66cc 100644 --- a/js/tex.jsx +++ b/js/tex.jsx @@ -2,8 +2,7 @@ /** * For math rendered using KaTex and/or MathJax. Use me like 2x + 3. */ -/* global MathJax, Khan */ -// TODO(joel) - require MathJax / katex so they don't have to be global +/* global MathJax */ const PureRenderMixin = require("react-addons-pure-render-mixin"); const React = require("react"); @@ -39,10 +38,24 @@ const unProcess = script => { const loadMathJax = callback => { if (typeof MathJax !== "undefined") { callback(); - } else if (typeof Khan !== "undefined" && Khan.mathJaxLoaded) { - Khan.mathJaxLoaded.then(callback); } else { - throw new Error("MathJax wasn't loaded before it was needed by "); + /** + * We can either... + * + * A) Reach up and out of `third_party` folder into our core webapp + * code to include this `load-mathjax` file that, well, loads MathJax + * or... + * + * B) Move this file into the KateX package next to `load-mathjax` and + * then update everything that calls for this file to point at that + * path instead. + * + * There is no easy or obvious solution. I am choosing A. + * + * - Jesse + */ + const loadMathJax = require("../../../../javascript/katex-package/load-mathjax.js"); + loadMathJax.then(callback); } }; @@ -98,15 +111,6 @@ const TeX = createReactClass({ mixins: [PureRenderMixin], - // TODO(joshuan): Once we are using React 16.3+, - // migrate to getDerivedStateFromProps - getInitialState: function() { - return { - mounted: false, - katexHtml: this.getKatexHtml(this.props), - }; - }, - getDefaultProps: function() { return { katexOptions: { @@ -124,6 +128,15 @@ const TeX = createReactClass({ }; }, + // TODO(joshuan): Once we are using React 16.3+, + // migrate to getDerivedStateFromProps + getInitialState: function() { + return { + mounted: false, + katexHtml: this.getKatexHtml(this.props), + }; + }, + componentDidMount: function() { this._root = ReactDOM.findDOMNode(this); @@ -160,27 +173,6 @@ const TeX = createReactClass({ } }, - getKatexHtml(props) { - // Try to render the math content with KaTeX. - // If this fails, componentDidUpdate() will notice and - // use MathJAX instead. - try { - return { - __html: katex.renderToString( - props.children, - props.katexOptions, - ), - }; - } catch (e) { - /* jshint -W103 */ - if (e.__proto__ !== katex.ParseError.prototype) { - /* jshint +W103 */ - throw e; - } - return null; - } - }, - componentDidUpdate: function(prevProps, prevState) { if (this.props.children !== prevProps.children) { this.maybeUnprocess(); @@ -237,6 +229,27 @@ const TeX = createReactClass({ } }, + getKatexHtml(props) { + // Try to render the math content with KaTeX. + // If this fails, componentDidUpdate() will notice and + // use MathJAX instead. + try { + return { + __html: katex.renderToString( + props.children, + props.katexOptions, + ), + }; + } catch (e) { + /* jshint -W103 */ + if (e.__proto__ !== katex.ParseError.prototype) { + /* jshint +W103 */ + throw e; + } + return null; + } + }, + process: function(callback) { this.hasProcessed = false; process(this.script, () => { From a9315ce68a8a5d2afafe42f965988cbb10ce10f6 Mon Sep 17 00:00:00 2001 From: David Flanagan Date: Fri, 9 Nov 2018 15:34:38 -0800 Subject: [PATCH 2/2] Fallback to MathJAX on any Katex error, not just parse errors Summary: Bugs CP-879 and CP-1742 are caused by a TypeError thrown by KaTeX. We already fall back to MathJAX rendering when KaTeX throws a parse error. But currently we rethrow other kinds of errors. This patch changes that so we fallback to MathJAX for any kind of KaTeX error. The underlying KaTeX error has been fixed upstream and will go away when we upgrade webapp to use KaTeX 1.0. Since we've got the MathJAX fallback, however, it seems worth using it in case other errors arise in the future. Test Plan: Visit the following URL with this patch and verifies that it renders: http://localhost:8081/math/k-8-grades/fr-v2-terminale-s/fr-v2-matrices-enseignement-de-spcialit/fr-v2-oprations-sur-les-matrices/a/multiplying-matrices-by-scalars?lang=fr Reviewers: kevinb, michaelpolyak Reviewed By: kevinb Subscribers: tom Differential Revision: https://phabricator.khanacademy.org/D50139 --- js/tex.jsx | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/js/tex.jsx b/js/tex.jsx index c7f66cc..d4c375b 100644 --- a/js/tex.jsx +++ b/js/tex.jsx @@ -241,11 +241,24 @@ const TeX = createReactClass({ ), }; } catch (e) { - /* jshint -W103 */ - if (e.__proto__ !== katex.ParseError.prototype) { - /* jshint +W103 */ - throw e; - } + // By catching the exception here and returning null + // we will fall back to asyncronously rendering with + // MathJAX. + // + // NOTE: formerly we only returned null if the error + // was a parse error from Katex and re-threw any other errors. + // But https://khanacademy.atlassian.net/browse/CP-879 and + // https://khanacademy.atlassian.net/browse/CP-1742 were caused + // by regular TypeError exceptions in Katex, so we might as + // well fall back to MathJAX in that case as well. (The Katex + // bug is fixed in the latest version and will stop happening + // when we upgrade webapp to use Katex 1.0.) + // + // TODO: We could use Raven.captureMessage() to send a message + // to Sentry when these errors occur if we want to get serious + // about eliminating them. Such a message should include + // window.location, props.children (the string of katex source) + // and the error itself. return null; } },