Skip to content

Commit

Permalink
Fix bug with (injected) custom elements and layouts
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
3 people committed Oct 11, 2022
1 parent 8970e21 commit 90fa493
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 12 deletions.
30 changes: 18 additions & 12 deletions packages/mdx/lib/plugin/recma-jsx-rewrite.js
Expand Up @@ -32,6 +32,7 @@
* @property {Array<string>} components
* @property {Array<string>} tags
* @property {Record<string, {node: JSXElement, component: boolean}>} references
* @property {Map<string|number, string>} idToInvalidComponentName
* @property {ESFunction} node
*/

Expand Down Expand Up @@ -72,8 +73,6 @@ export function recmaJsxRewrite(options = {}) {
let createErrorHelper
/** @type {Scope|null} */
let currentScope
/** @type {Map<string | number, string>} */
const idToInvalidComponentName = new Map()

walk(tree, {
enter(_node) {
Expand All @@ -92,6 +91,7 @@ export function recmaJsxRewrite(options = {}) {
components: [],
tags: [],
references: {},
idToInvalidComponentName: new Map(),
node
})

Expand Down Expand Up @@ -198,10 +198,11 @@ export function recmaJsxRewrite(options = {}) {
/** @type {Array<string | number>} */
let jsxIdExpression = ['_components', id]
if (isIdentifierName(id) === false) {
let invalidComponentName = idToInvalidComponentName.get(id)
let invalidComponentName =
fnScope.idToInvalidComponentName.get(id)
if (invalidComponentName === undefined) {
invalidComponentName = `_component${idToInvalidComponentName.size}`
idToInvalidComponentName.set(id, invalidComponentName)
invalidComponentName = `_component${fnScope.idToInvalidComponentName.size}`
fnScope.idToInvalidComponentName.set(id, invalidComponentName)
}

jsxIdExpression = [invalidComponentName]
Expand Down Expand Up @@ -272,7 +273,7 @@ export function recmaJsxRewrite(options = {}) {
if (
defaults.length > 0 ||
actual.length > 0 ||
idToInvalidComponentName.size > 0
scope.idToInvalidComponentName.size > 0
) {
if (providerImportSource) {
importProvider = true
Expand Down Expand Up @@ -359,7 +360,10 @@ export function recmaJsxRewrite(options = {}) {
}

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

statements.push({
type: 'VariableDeclaration',
kind: 'const',
declarations
})
if (declarations.length > 0) {
statements.push({
type: 'VariableDeclaration',
kind: 'const',
declarations
})
}
}

/** @type {string} */
Expand Down
29 changes: 29 additions & 0 deletions packages/mdx/test/compile.js
Expand Up @@ -1173,6 +1173,35 @@ test('remark-rehype options', async () => {
)
})

// See <https://github.com/mdx-js/mdx/issues/2112>
test('should support custom elements with layouts', async () => {
assert.equal(
renderToStaticMarkup(
React.createElement(
await run(
await compile('export default function () {}', {
rehypePlugins: [
/** @type {import('unified').Plugin<[], import('hast').Root>} */
function () {
return function (tree) {
tree.children.push({
type: 'element',
tagName: 'custom-element',
properties: {},
children: []
})
}
}
]
})
)
)
),
'',
'should not crash if element names are used that are not valid JavaScript identifiers, with layouts'
)
})

test('MDX (JSX)', async () => {
assert.equal(
renderToStaticMarkup(
Expand Down

1 comment on commit 90fa493

@vercel
Copy link

@vercel vercel bot commented on 90fa493 Oct 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.