Merge pull request #212 from marp-team/math-context-handling

Math plugin: Better context handling for defined macro

CHANGELOG.md

@@ -2,6 +2,11 @@
 
 ## [Unreleased]
 
+### Fixed
+
+- KaTeX: Persist defined global macro between math renderings ([#212](https://github.com/marp-team/marp-core/pull/212))
+- MathJax: Prevent leaking defined macro between Markdown renderings ([#212](https://github.com/marp-team/marp-core/pull/212))
+
 ### Changed
 
 - Upgrade Marpit to [v1.6.4](https://github.com/marp-team/marpit/releases/v1.6.4) ([#210](https://github.com/marp-team/marp-core/pull/210))

src/math/context.ts

@@ -0,0 +1,26 @@
+import type { MathOptionsInterface } from './math'
+
+type MathContext = {
+  enabled: boolean
+  options: MathOptionsInterface
+
+  // Library specific contexts
+  katexMacroContext: Record<string, string>
+  mathjaxContext: any
+}
+
+const contextSymbol = Symbol('marp-math-context')
+
+export const setMathContext = (
+  target: any,
+  setter: (current: MathContext) => MathContext
+) => {
+  if (!Object.prototype.hasOwnProperty.call(target, contextSymbol)) {
+    Object.defineProperty(target, contextSymbol, { writable: true })
+  }
+  target[contextSymbol] = setter(target[contextSymbol])
+}
+
+export const getMathContext = (target: any): MathContext => ({
+  ...target[contextSymbol],
+})

src/math/katex.ts

@@ -1,17 +1,23 @@
 import katex from 'katex'
 import { version } from 'katex/package.json'
+import { getMathContext } from './context'
 import katexScss from './katex.scss'
 
 const convertedCSS = Object.create(null)
 const katexMatcher = /url\(['"]?fonts\/(.*?)['"]?\)/g
 
-export const inline = (opts: Record<string, unknown> = {}) => (tokens, idx) => {
+export const inline = (marpit: any) => (tokens, idx) => {
   const { content } = tokens[idx]
+  const {
+    options: { katexOption },
+    katexMacroContext,
+  } = getMathContext(marpit)
 
   try {
     return katex.renderToString(content, {
       throwOnError: false,
-      ...opts,
+      ...(katexOption || {}),
+      macros: katexMacroContext,
       displayMode: false,
     })
   } catch (e) {
@@ -20,13 +26,18 @@ export const inline = (opts: Record<string, unknown> = {}) => (tokens, idx) => {
   }
 }
 
-export const block = (opts: Record<string, unknown> = {}) => (tokens, idx) => {
+export const block = (marpit: any) => (tokens, idx) => {
   const { content } = tokens[idx]
+  const {
+    options: { katexOption },
+    katexMacroContext,
+  } = getMathContext(marpit)
 
   try {
     return `<p>${katex.renderToString(content, {
       throwOnError: false,
-      ...opts,
+      ...(katexOption || {}),
+      macros: katexMacroContext,
       displayMode: true,
     })}</p>`
   } catch (e) {

src/math/math.ts

@@ -1,15 +1,14 @@
 import marpitPlugin from '@marp-team/marpit/plugin'
+import { getMathContext, setMathContext } from './context'
 import * as katex from './katex'
 import * as mathjax from './mathjax'
 
-interface MathOptionsInterface {
+export interface MathOptionsInterface {
   lib?: 'katex' | 'mathjax'
   katexOption?: Record<string, unknown>
   katexFontPath?: string | false
 }
 
-const contextSymbol = Symbol('marp-math-context')
-
 export type MathOptions =
   | boolean
   | MathOptionsInterface['lib']
@@ -24,52 +23,70 @@ export const markdown = marpitPlugin((md) => {
       ? { lib: typeof opts === 'string' ? opts : undefined }
       : opts
 
-  Object.defineProperty(md.marpit, contextSymbol, { writable: true })
+  // Initialize
+  const { parse, parseInline } = md
+
+  const initializeMathContext = () =>
+    setMathContext(md.marpit, () => ({
+      enabled: false,
+      options: parsedOpts,
+      katexMacroContext: {
+        ...((parsedOpts.katexOption?.macros as any) || {}),
+      },
+      mathjaxContext: null,
+    }))
+
+  md.parse = function (...args) {
+    initializeMathContext()
+    return parse.apply(this, args)
+  }
 
-  md.core.ruler.before('block', 'marp_math_initialize', ({ inlineMode }) => {
-    if (!inlineMode) md.marpit[contextSymbol] = null
-  })
+  md.parseInline = function (...args) {
+    initializeMathContext()
+    return parseInline.apply(this, args)
+  }
+
+  const enableMath = () =>
+    setMathContext(md.marpit, (ctx) => ({ ...ctx, enabled: true }))
 
   // Inline
   md.inline.ruler.after('escape', 'marp_math_inline', (state, silent) => {
-    if (parseInlineMath(state, silent)) {
-      md.marpit[contextSymbol] = parsedOpts
-      return true
-    }
-    return false
+    const ret = parseInlineMath(state, silent)
+    if (ret) enableMath()
+
+    return ret
   })
 
   // Block
   md.block.ruler.after(
     'blockquote',
     'marp_math_block',
     (state, start, end, silent) => {
-      if (parseMathBlock(state, start, end, silent)) {
-        md.marpit[contextSymbol] = parsedOpts
-        return true
-      }
-      return false
+      const ret = parseMathBlock(state, start, end, silent)
+      if (ret) enableMath()
+
+      return ret
     },
     { alt: ['paragraph', 'reference', 'blockquote', 'list'] }
   )
 
   // Renderer
   if (parsedOpts.lib === 'mathjax') {
-    md.renderer.rules.marp_math_inline = mathjax.inline()
-    md.renderer.rules.marp_math_block = mathjax.block()
+    md.renderer.rules.marp_math_inline = mathjax.inline(md.marpit)
+    md.renderer.rules.marp_math_block = mathjax.block(md.marpit)
   } else {
-    md.renderer.rules.marp_math_inline = katex.inline(parsedOpts.katexOption)
-    md.renderer.rules.marp_math_block = katex.block(parsedOpts.katexOption)
+    md.renderer.rules.marp_math_inline = katex.inline(md.marpit)
+    md.renderer.rules.marp_math_block = katex.block(md.marpit)
   }
 })
 
 export const css = (marpit: any): string | null => {
-  const opts: MathOptionsInterface | null = marpit[contextSymbol]
-  if (!opts) return null
+  const { enabled, options } = getMathContext(marpit)
+  if (!enabled) return null
 
-  if (opts.lib === 'mathjax') return mathjax.css()
+  if (options.lib === 'mathjax') return mathjax.css(marpit)
 
-  return katex.css(opts.katexFontPath)
+  return katex.css(options.katexFontPath)
 }
 
 function isValidDelim(state, pos = state.pos) {

src/math/mathjax.ts

@@ -4,17 +4,18 @@ import { TeX } from 'mathjax-full/js/input/tex'
 import { AllPackages } from 'mathjax-full/js/input/tex/AllPackages'
 import { mathjax } from 'mathjax-full/js/mathjax'
 import { SVG } from 'mathjax-full/js/output/svg'
+import { getMathContext, setMathContext } from './context'
 
 interface MathJaxContext {
   adaptor: LiteAdaptor
   css: string
   document: ReturnType<typeof mathjax['document']>
 }
 
-let lazyContext: MathJaxContext | undefined
+const context = (marpit: any): MathJaxContext => {
+  let { mathjaxContext } = getMathContext(marpit)
 
-const context = (): MathJaxContext => {
-  if (!lazyContext) {
+  if (!mathjaxContext) {
     const adaptor = liteAdaptor()
     RegisterHTMLHandler(adaptor)
 
@@ -23,13 +24,15 @@ const context = (): MathJaxContext => {
     const document = mathjax.document('', { InputJax: tex, OutputJax: svg })
     const css = adaptor.textContent(svg.styleSheet(document) as any)
 
-    lazyContext = { adaptor, css, document }
+    mathjaxContext = { adaptor, css, document }
+    setMathContext(marpit, (ctx) => ({ ...ctx, mathjaxContext }))
   }
-  return lazyContext
+
+  return mathjaxContext
 }
 
-export const inline = () => (tokens, idx) => {
-  const { adaptor, document } = context()
+export const inline = (marpit: any) => (tokens, idx) => {
+  const { adaptor, document } = context(marpit)
   const { content } = tokens[idx]
 
   try {
@@ -40,10 +43,10 @@ export const inline = () => (tokens, idx) => {
   }
 }
 
-export const block = () =>
+export const block = (marpit: any) =>
   Object.assign(
     (tokens, idx) => {
-      const { adaptor, document } = context()
+      const { adaptor, document } = context(marpit)
       const { content } = tokens[idx]
 
       try {
@@ -66,4 +69,4 @@ export const block = () =>
     { scaled: true }
   )
 
-export const css = () => context().css
+export const css = (marpit: any) => context(marpit).css

test/marp.ts

@@ -347,6 +347,31 @@ describe('Marp', () => {
         expect(katexFonts).toMatchSnapshot('katex-css-cdn')
       })
 
+      it('has a unique context for macro by Markdown rendering', () => {
+        const instance = marp()
+
+        const plain = cheerio
+          .load(instance.render('$x^2$').html)('.katex-html')
+          .html()
+
+        // KaTeX can modify macros through \gdef
+        const globallyDefined = cheerio
+          .load(instance.render('$\\gdef\\foo{x^2}$ $\\foo$').html)(
+            '.katex-html'
+          )
+          .eq(1)
+          .html()
+
+        expect(globallyDefined).toBe(plain)
+
+        // Defined command through \gdef in another rendering cannot use
+        const notDefined = cheerio
+          .load(instance.render('$\\foo$').html)('.katex-html')
+          .html()
+
+        expect(notDefined).not.toBe(plain)
+      })
+
       describe('when math typesetting syntax is not using', () => {
         it('does not inject KaTeX css', () =>
           expect(marp().render('plain text').css).not.toContain('.katex'))
@@ -440,6 +465,30 @@ describe('Marp', () => {
         expect(css).toContain('mjx-container')
       })
 
+      it('has a unique context for macro by Markdown rendering', () => {
+        const instance = marp({ math: 'mathjax' })
+
+        const plain = cheerio
+          .load(instance.render('$x^2$').html)('mjx-container')
+          .html()
+
+        const defined = cheerio
+          .load(instance.render('$\\def\\foo{x^2}$ $\\foo$').html)(
+            'mjx-container'
+          )
+          .eq(1)
+          .html()
+
+        expect(defined).toBe(plain)
+
+        // Defined command through \def in another rendering cannot use
+        const notDefined = cheerio
+          .load(instance.render('$\\foo$').html)('mjx-container')
+          .html()
+
+        expect(notDefined).not.toBe(plain)
+      })
+
       describe('when math typesetting syntax is not using', () => {
         it('does not inject MathJax css', () =>
           expect(