Skip to content

Commit 62931c2

Browse files
authored
Refactor @mdx-js/mdx to clean code, test 100%
Code changes: * Remove footnotes by default: if folks want that, they can add `remark-footnotes` themselves in `remarkPlugins` * Remove undocumented and untested `options.compilers` * Remove undocumented and untested `raw` node handling. The way it was done would result in bugs. If folks want HTML in MDX, they can use `rehype-raw` in `rehypePlugins` * Defer plugin handling to `.use` * Remove unused dependencies * Refactor code Tests: * Make test cases smaller so that unrelated changes cause less failures * Where possible, compare MDX output to JSX, so that stylistic differences are ignored, and our baseline is met * Where possible, evaluate the MDX, to check that core, components, plugins, all actually work together * Remove unneeded tests, such as where covered by `remark-mdx` or `remark-mdxjs` * Add tests for everything that’s exposed by this package * 100% coverage Closes GH-1347. Reviewed-by: Christian Murphy <[email protected]>
1 parent 5d08233 commit 62931c2

File tree

13 files changed

+832
-898
lines changed

13 files changed

+832
-898
lines changed

packages/mdx/.babelrc

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"presets": [
3+
[
4+
"@babel/env",
5+
{
6+
"corejs": 3,
7+
"useBuiltIns": "usage"
8+
}
9+
],
10+
"@babel/react"
11+
]
12+
}

packages/mdx/index.js

Lines changed: 27 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -1,120 +1,51 @@
11
const unified = require('unified')
2-
const toMDAST = require('remark-parse')
2+
const remarkParse = require('remark-parse')
33
const remarkMdx = require('remark-mdx')
44
const remarkMdxJs = require('remark-mdxjs')
5-
const footnotes = require('remark-footnotes')
65
const squeeze = require('remark-squeeze-paragraphs')
7-
const visit = require('unist-util-visit')
8-
const raw = require('hast-util-raw')
96
const mdxAstToMdxHast = require('./mdx-ast-to-mdx-hast')
107
const mdxHastToJsx = require('./mdx-hast-to-jsx')
118

12-
const DEFAULT_OPTIONS = {
13-
remarkPlugins: [],
14-
rehypePlugins: [],
15-
compilers: []
16-
}
17-
18-
function createMdxAstCompiler(options) {
19-
const plugins = options.remarkPlugins
20-
21-
const fn = unified()
22-
.use(toMDAST, options)
23-
.use(remarkMdx, options)
24-
.use(remarkMdxJs, options)
25-
.use(footnotes, options)
26-
.use(squeeze, options)
27-
28-
plugins.forEach(plugin => {
29-
// Handle [plugin, pluginOptions] syntax
30-
if (Array.isArray(plugin) && plugin.length > 1) {
31-
fn.use(plugin[0], plugin[1])
32-
} else {
33-
fn.use(plugin)
34-
}
35-
})
9+
const pragma = `/* @jsx mdx */`
3610

37-
fn.use(mdxAstToMdxHast, options)
38-
39-
return fn
40-
}
41-
42-
function applyHastPluginsAndCompilers(compiler, options) {
43-
const plugins = options.rehypePlugins
44-
45-
const compilers = options.compilers
46-
47-
// Convert raw nodes into HAST
48-
compiler.use(() => ast => {
49-
visit(ast, 'raw', node => {
50-
const {children, tagName, properties} = raw(node)
51-
node.type = 'element'
52-
node.children = children
53-
node.tagName = tagName
54-
55-
node.properties = properties
56-
})
57-
})
58-
59-
plugins.forEach(plugin => {
60-
// Handle [plugin, pluginOptions] syntax
61-
if (Array.isArray(plugin) && plugin.length > 1) {
62-
compiler.use(plugin[0], plugin[1])
63-
} else {
64-
compiler.use(plugin)
65-
}
66-
})
67-
68-
compiler.use(mdxHastToJsx, options)
69-
70-
for (const compilerPlugin of compilers) {
71-
compiler.use(compilerPlugin, options)
72-
}
73-
74-
return compiler
11+
function createMdxAstCompiler(options = {}) {
12+
return unified()
13+
.use(remarkParse)
14+
.use(remarkMdx)
15+
.use(remarkMdxJs)
16+
.use(squeeze)
17+
.use(options.remarkPlugins)
18+
.use(mdxAstToMdxHast)
7519
}
7620

7721
function createCompiler(options = {}) {
78-
const opts = Object.assign({}, DEFAULT_OPTIONS, options)
79-
const compiler = createMdxAstCompiler(opts)
80-
const compilerWithPlugins = applyHastPluginsAndCompilers(compiler, opts)
81-
82-
return compilerWithPlugins
22+
return createMdxAstCompiler(options)
23+
.use(options.rehypePlugins)
24+
.use(mdxHastToJsx, options)
8325
}
8426

85-
function sync(mdx, options = {}) {
86-
const compiler = createCompiler(options)
27+
function createConfig(mdx, options) {
28+
const config = {contents: mdx}
8729

88-
const fileOpts = {contents: mdx}
8930
if (options.filepath) {
90-
fileOpts.path = options.filepath
31+
config.path = options.filepath
9132
}
9233

93-
const {contents} = compiler.processSync(fileOpts)
34+
return config
35+
}
9436

95-
return `/* @jsx mdx */
96-
${contents}`
37+
function sync(mdx, options = {}) {
38+
const file = createCompiler(options).processSync(createConfig(mdx, options))
39+
return pragma + '\n' + String(file)
9740
}
9841

9942
async function compile(mdx, options = {}) {
100-
const compiler = createCompiler(options)
101-
102-
const fileOpts = {contents: mdx}
103-
if (options.filepath) {
104-
fileOpts.path = options.filepath
105-
}
106-
107-
const {contents} = await compiler.process(fileOpts)
108-
109-
return `/* @jsx mdx */
110-
${contents}`
43+
const file = await createCompiler(options).process(createConfig(mdx, options))
44+
return pragma + '\n' + String(file)
11145
}
11246

113-
compile.sync = sync
114-
11547
module.exports = compile
116-
exports = compile
117-
exports.sync = sync
118-
exports.createMdxAstCompiler = createMdxAstCompiler
119-
exports.createCompiler = createCompiler
120-
exports.default = compile
48+
compile.default = compile
49+
compile.sync = sync
50+
compile.createMdxAstCompiler = createMdxAstCompiler
51+
compile.createCompiler = createCompiler

packages/mdx/mdx-ast-to-mdx-hast.js

Lines changed: 63 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -1,102 +1,80 @@
1-
const toHAST = require('mdast-util-to-hast')
1+
const toHast = require('mdast-util-to-hast')
22
const all = require('mdast-util-to-hast/lib/all')
33
const detab = require('detab')
44
const u = require('unist-builder')
55

66
function mdxAstToMdxHast() {
7-
return (tree, _file) => {
8-
const handlers = {
9-
// `inlineCode` gets passed as `code` by the HAST transform.
10-
// This makes sure it ends up being `inlineCode`
11-
inlineCode(h, node) {
12-
return Object.assign({}, node, {
13-
type: 'element',
14-
tagName: 'inlineCode',
15-
properties: {},
16-
children: [
17-
{
18-
type: 'text',
19-
value: node.value
20-
}
21-
]
22-
})
23-
},
24-
code(h, node) {
25-
const value = node.value ? detab(node.value + '\n') : ''
26-
const lang = node.lang
27-
const props = {}
7+
return tree => {
8+
return toHast(tree, {
9+
handlers: {
10+
// `inlineCode` gets passed as `code` by the HAST transform.
11+
// This makes sure it ends up being `inlineCode`
12+
inlineCode(h, node) {
13+
return h(node, 'inlineCode', [{type: 'text', value: node.value}])
14+
},
15+
code(h, node) {
16+
const value = node.value ? detab(node.value + '\n') : ''
17+
const lang = node.lang
18+
const props = {}
2819

29-
if (lang) {
30-
props.className = ['language-' + lang]
31-
}
20+
if (lang) {
21+
props.className = ['language-' + lang]
22+
}
23+
24+
props.metastring = node.meta
3225

33-
// MDAST sets `node.meta` to `null` instead of `undefined` if
34-
// not present, which React doesn't like.
35-
props.metastring = node.meta || undefined
26+
// To do: this handling seems not what users expect:
27+
// <https://github.com/mdx-js/mdx/issues/702>.
28+
const meta =
29+
node.meta &&
30+
node.meta.split(' ').reduce((acc, cur) => {
31+
if (cur.split('=').length > 1) {
32+
const t = cur.split('=')
33+
acc[t[0]] = t[1]
34+
return acc
35+
}
3636

37-
const meta =
38-
node.meta &&
39-
node.meta.split(' ').reduce((acc, cur) => {
40-
if (cur.split('=').length > 1) {
41-
const t = cur.split('=')
42-
acc[t[0]] = t[1]
37+
acc[cur] = true
4338
return acc
44-
}
39+
}, {})
4540

46-
acc[cur] = true
47-
return acc
48-
}, {})
41+
if (meta) {
42+
Object.keys(meta).forEach(key => {
43+
const isClassKey = key === 'class' || key === 'className'
44+
if (props.className && isClassKey) {
45+
props.className.push(meta[key])
46+
} else {
47+
props[key] = meta[key]
48+
}
49+
})
50+
}
4951

50-
if (meta) {
51-
Object.keys(meta).forEach(key => {
52-
const isClassKey = key === 'class' || key === 'className'
53-
if (props.className && isClassKey) {
54-
props.className.push(meta[key])
55-
} else {
56-
props[key] = meta[key]
57-
}
58-
})
52+
return h(node.position, 'pre', [
53+
h(node, 'code', props, [u('text', value)])
54+
])
55+
},
56+
// To do: rename to `mdxJsImport`
57+
import(h, node) {
58+
return Object.assign({}, node, {type: 'import'})
59+
},
60+
// To do: rename to `mdxJsExport`
61+
export(h, node) {
62+
return Object.assign({}, node, {type: 'export'})
63+
},
64+
mdxBlockElement(h, node) {
65+
return Object.assign({}, node, {children: all(h, node)})
66+
},
67+
mdxSpanElement(h, node) {
68+
return Object.assign({}, node, {children: all(h, node)})
69+
},
70+
mdxBlockExpression(h, node) {
71+
return Object.assign({}, node, {type: 'mdxBlockExpression'})
72+
},
73+
mdxSpanExpression(h, node) {
74+
return Object.assign({}, node, {type: 'mdxSpanExpression'})
5975
}
60-
61-
return h(node.position, 'pre', [
62-
h(node, 'code', props, [u('text', value)])
63-
])
64-
},
65-
// To do: rename to `mdxJsImport`
66-
import(h, node) {
67-
return Object.assign({}, node, {
68-
type: 'import'
69-
})
70-
},
71-
// To do: rename to `mdxJsExport`
72-
export(h, node) {
73-
return Object.assign({}, node, {
74-
type: 'export'
75-
})
76-
},
77-
mdxBlockElement(h, node) {
78-
return Object.assign({}, node, {children: all(h, node)})
79-
},
80-
mdxSpanElement(h, node) {
81-
return Object.assign({}, node, {children: all(h, node)})
82-
},
83-
mdxBlockExpression(h, node) {
84-
return Object.assign({}, node, {
85-
type: 'mdxBlockExpression'
86-
})
87-
},
88-
mdxSpanExpression(h, node) {
89-
return Object.assign({}, node, {
90-
type: 'mdxSpanExpression'
91-
})
9276
}
93-
}
94-
95-
const hast = toHAST(tree, {
96-
handlers
9777
})
98-
99-
return hast
10078
}
10179
}
10280

0 commit comments

Comments
 (0)