Skip to content

Commit 90fa493

Browse files
wooormcalebebybholmesdev
committedOct 11, 2022
Fix bug with (injected) custom elements and layouts
This fixes a bug, uncovered in #2112 and in #2123, which is rather unlikely to occur. It only occurs when: 1. Custom elements (such as `<a-b></a-b>`) are injected (not authored) into a tree 2. Either: 1. A layout component is defined within an MDX document 2. A provider is used, and any component is defined within MDX documents In those cases, an accidental `const ;` was injected into the final serialized document. Which caused anything trying to run the code to crash. The problem was introduced in 9904838, the commit message of which sheds some light on why custom elements are peculiar and need extra handling. We track which component contains which other components. If some component uses `<A />`, then some code to handle `A` is injected in that component. If a different component uses `<B />`, some code for `B` is injected inside it. But the components don’t need to know about what’s used in other components. This mechanism had a mistake for custom elements: they were tracked globally. This commit fixes that, by tracking them scoped to the component that includes them. Related-to: GH-2100. Related-to: GH-2101. Closes GH-2112. Closes GH-2123. Co-authored-by: Caleb Eby <caleb.eby01@gmail.com> Co-authored-by: bholmesdev <hey@bholmes.dev>
1 parent 8970e21 commit 90fa493

File tree

2 files changed

+47
-12
lines changed

2 files changed

+47
-12
lines changed
 

Diff for: ‎packages/mdx/lib/plugin/recma-jsx-rewrite.js

+18-12
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
* @property {Array<string>} components
3333
* @property {Array<string>} tags
3434
* @property {Record<string, {node: JSXElement, component: boolean}>} references
35+
* @property {Map<string|number, string>} idToInvalidComponentName
3536
* @property {ESFunction} node
3637
*/
3738

@@ -72,8 +73,6 @@ export function recmaJsxRewrite(options = {}) {
7273
let createErrorHelper
7374
/** @type {Scope|null} */
7475
let currentScope
75-
/** @type {Map<string | number, string>} */
76-
const idToInvalidComponentName = new Map()
7776

7877
walk(tree, {
7978
enter(_node) {
@@ -92,6 +91,7 @@ export function recmaJsxRewrite(options = {}) {
9291
components: [],
9392
tags: [],
9493
references: {},
94+
idToInvalidComponentName: new Map(),
9595
node
9696
})
9797

@@ -198,10 +198,11 @@ export function recmaJsxRewrite(options = {}) {
198198
/** @type {Array<string | number>} */
199199
let jsxIdExpression = ['_components', id]
200200
if (isIdentifierName(id) === false) {
201-
let invalidComponentName = idToInvalidComponentName.get(id)
201+
let invalidComponentName =
202+
fnScope.idToInvalidComponentName.get(id)
202203
if (invalidComponentName === undefined) {
203-
invalidComponentName = `_component${idToInvalidComponentName.size}`
204-
idToInvalidComponentName.set(id, invalidComponentName)
204+
invalidComponentName = `_component${fnScope.idToInvalidComponentName.size}`
205+
fnScope.idToInvalidComponentName.set(id, invalidComponentName)
205206
}
206207

207208
jsxIdExpression = [invalidComponentName]
@@ -272,7 +273,7 @@ export function recmaJsxRewrite(options = {}) {
272273
if (
273274
defaults.length > 0 ||
274275
actual.length > 0 ||
275-
idToInvalidComponentName.size > 0
276+
scope.idToInvalidComponentName.size > 0
276277
) {
277278
if (providerImportSource) {
278279
importProvider = true
@@ -359,7 +360,10 @@ export function recmaJsxRewrite(options = {}) {
359360
}
360361

361362
if (isNamedFunction(scope.node, '_createMdxContent')) {
362-
for (const [id, componentName] of idToInvalidComponentName) {
363+
for (const [
364+
id,
365+
componentName
366+
] of scope.idToInvalidComponentName) {
363367
// For JSX IDs that can’t be represented as JavaScript IDs (as in,
364368
// those with dashes, such as `custom-element`), generate a
365369
// separate variable that is a valid JS ID (such as `_component0`),
@@ -387,11 +391,13 @@ export function recmaJsxRewrite(options = {}) {
387391
})
388392
}
389393

390-
statements.push({
391-
type: 'VariableDeclaration',
392-
kind: 'const',
393-
declarations
394-
})
394+
if (declarations.length > 0) {
395+
statements.push({
396+
type: 'VariableDeclaration',
397+
kind: 'const',
398+
declarations
399+
})
400+
}
395401
}
396402

397403
/** @type {string} */

Diff for: ‎packages/mdx/test/compile.js

+29
Original file line numberDiff line numberDiff line change
@@ -1173,6 +1173,35 @@ test('remark-rehype options', async () => {
11731173
)
11741174
})
11751175

1176+
// See <https://github.com/mdx-js/mdx/issues/2112>
1177+
test('should support custom elements with layouts', async () => {
1178+
assert.equal(
1179+
renderToStaticMarkup(
1180+
React.createElement(
1181+
await run(
1182+
await compile('export default function () {}', {
1183+
rehypePlugins: [
1184+
/** @type {import('unified').Plugin<[], import('hast').Root>} */
1185+
function () {
1186+
return function (tree) {
1187+
tree.children.push({
1188+
type: 'element',
1189+
tagName: 'custom-element',
1190+
properties: {},
1191+
children: []
1192+
})
1193+
}
1194+
}
1195+
]
1196+
})
1197+
)
1198+
)
1199+
),
1200+
'',
1201+
'should not crash if element names are used that are not valid JavaScript identifiers, with layouts'
1202+
)
1203+
})
1204+
11761205
test('MDX (JSX)', async () => {
11771206
assert.equal(
11781207
renderToStaticMarkup(

1 commit comments

Comments
 (1)

vercel[bot] commented on Oct 11, 2022

@vercel[bot]

Successfully deployed to the following URLs:

mdx – ./

mdx-mdx.vercel.app
mdx-git-main-mdx.vercel.app
mdxjs.com
v2.mdxjs.com

Please sign in to comment.