From 90fa4935d23cb206cdd181082687c102db9eaa64 Mon Sep 17 00:00:00 2001
From: Titus Wormer <tituswormer@gmail.com>
Date: Tue, 11 Oct 2022 19:04:31 +0200
Subject: [PATCH] Fix bug with (injected) custom elements and layouts
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This fixes a bug, uncovered in mdx-js/mdx#2112 and in mdx-js/mdx#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>
---
 packages/mdx/lib/plugin/recma-jsx-rewrite.js | 30 ++++++++++++--------
 packages/mdx/test/compile.js                 | 29 +++++++++++++++++++
 2 files changed, 47 insertions(+), 12 deletions(-)

diff --git a/packages/mdx/lib/plugin/recma-jsx-rewrite.js b/packages/mdx/lib/plugin/recma-jsx-rewrite.js
index 1e82a1e41..c9398a208 100644
--- a/packages/mdx/lib/plugin/recma-jsx-rewrite.js
+++ b/packages/mdx/lib/plugin/recma-jsx-rewrite.js
@@ -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
  */
 
@@ -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) {
@@ -92,6 +91,7 @@ export function recmaJsxRewrite(options = {}) {
             components: [],
             tags: [],
             references: {},
+            idToInvalidComponentName: new Map(),
             node
           })
 
@@ -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]
@@ -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
@@ -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`),
@@ -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} */
diff --git a/packages/mdx/test/compile.js b/packages/mdx/test/compile.js
index d5a10ee05..ea6ae715f 100644
--- a/packages/mdx/test/compile.js
+++ b/packages/mdx/test/compile.js
@@ -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(