From 225b9ceadad83df452256946ea32f707e24c6ae6 Mon Sep 17 00:00:00 2001 From: Remco Haszing Date: Sun, 25 Feb 2024 13:54:06 +0100 Subject: [PATCH] Always treat _createMdxContent as a JSX component MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the user provides a `wrapper` component, `_createMdxContent` was treated as a JSX component. Otherwise it was invoked as a function. Because `_createMdxContent` uses a hook, this hooks becomes part of the `MDXContent` component conditionally. This breaks React’s rule of hooks. Closes #2444 --- docs/docs/using-mdx.mdx | 2 +- docs/guides/injecting-components.mdx | 2 +- packages/mdx/lib/plugin/recma-document.js | 54 +++++++++++------------ packages/mdx/readme.md | 8 ++-- packages/mdx/test/compile.js | 16 +++---- packages/mdx/test/syntax.js | 14 ++++-- packages/rollup/test/index.js | 2 +- 7 files changed, 52 insertions(+), 46 deletions(-) diff --git a/docs/docs/using-mdx.mdx b/docs/docs/using-mdx.mdx index b990a8912..8a5e1e82e 100644 --- a/docs/docs/using-mdx.mdx +++ b/docs/docs/using-mdx.mdx @@ -78,7 +78,7 @@ export default function MDXContent(props = {}) { const {wrapper: MDXLayout} = props.components || {} return MDXLayout ? _jsx(MDXLayout, {...props, children: _jsx(_createMdxContent, {...props})}) - : _createMdxContent(props) + : _jsx(_createMdxContent, {...props}) } ``` diff --git a/docs/guides/injecting-components.mdx b/docs/guides/injecting-components.mdx index e3b52bc0e..89ae49dbe 100644 --- a/docs/guides/injecting-components.mdx +++ b/docs/guides/injecting-components.mdx @@ -78,7 +78,7 @@ changes when `providerImportSource: 'xxx'` is passed: + const {wrapper: MDXLayout} = {..._provideComponents(), ...props.components} return MDXLayout ? _jsx(MDXLayout, {...props, children: _jsx(_createMdxContent, {...props})}) - : _createMdxContent(props) + : _jsx(_createMdxContent, {...props}) ``` Observe that components have defaults (such as that `h1` will use `'h1'`) and diff --git a/packages/mdx/lib/plugin/recma-document.js b/packages/mdx/lib/plugin/recma-document.js index 0f7db1f6d..481aec567 100644 --- a/packages/mdx/lib/plugin/recma-document.js +++ b/packages/mdx/lib/plugin/recma-document.js @@ -559,8 +559,8 @@ export function recmaDocument(options) { * Functions. */ function createMdxContent(content, outputFormat, hasInternalLayout) { - /** @type {JSXElement} */ - const element = { + /** @type {Expression} */ + let result = { type: 'JSXElement', openingElement: { type: 'JSXOpeningElement', @@ -577,39 +577,15 @@ export function recmaDocument(options) { type: 'JSXClosingElement', name: {type: 'JSXIdentifier', name: 'MDXLayout'} }, - children: [ - { - type: 'JSXElement', - openingElement: { - type: 'JSXOpeningElement', - name: {type: 'JSXIdentifier', name: '_createMdxContent'}, - attributes: [ - { - type: 'JSXSpreadAttribute', - argument: {type: 'Identifier', name: 'props'} - } - ], - selfClosing: true - }, - closingElement: null, - children: [] - } - ] + children: [createMdxContentElement()] } - let result = /** @type {Expression} */ (element) - if (!hasInternalLayout) { result = { type: 'ConditionalExpression', test: {type: 'Identifier', name: 'MDXLayout'}, consequent: result, - alternate: { - type: 'CallExpression', - callee: {type: 'Identifier', name: '_createMdxContent'}, - arguments: [{type: 'Identifier', name: 'props'}], - optional: false - } + alternate: createMdxContentElement() } } @@ -693,6 +669,28 @@ export function recmaDocument(options) { } } +/** + * @returns {JSXElement} + */ +function createMdxContentElement() { + return { + type: 'JSXElement', + openingElement: { + type: 'JSXOpeningElement', + name: {type: 'JSXIdentifier', name: '_createMdxContent'}, + attributes: [ + { + type: 'JSXSpreadAttribute', + argument: {type: 'Identifier', name: 'props'} + } + ], + selfClosing: true + }, + closingElement: null, + children: [] + } +} + /** * @param {Program} tree * @param {string} name diff --git a/packages/mdx/readme.md b/packages/mdx/readme.md index 61cb4a46a..52c3c35dc 100644 --- a/packages/mdx/readme.md +++ b/packages/mdx/readme.md @@ -119,7 +119,7 @@ export default function MDXContent(props = {}) { const {wrapper: MDXLayout} = props.components || {} return MDXLayout ? _jsx(MDXLayout, {...props, children: _jsx(_createMdxContent, {...props})}) - : _createMdxContent(props) + : _jsx(_createMdxContent, {...props}) } ``` @@ -678,7 +678,7 @@ Configuration for `createProcessor` (TypeScript type). - children: _jsx(_createMdxContent, props) - }) + ? <_createMdxContent {...props} /> - : _createMdxContent(props) + : _jsx(_createMdxContent, {...props}) } } ``` @@ -897,8 +897,8 @@ Configuration for `createProcessor` (TypeScript type). + } return MDXLayout - ? _jsx(MDXLayout, {...props, children: _jsx(_createMdxContent, {})}) - : _createMdxContent() + ? _jsx(MDXLayout, {...props, children: _jsx(_createMdxContent, {...props})}) + : _jsx(_createMdxContent, {...props}) ``` diff --git a/packages/mdx/test/compile.js b/packages/mdx/test/compile.js index 554082a3b..176095f8a 100644 --- a/packages/mdx/test/compile.js +++ b/packages/mdx/test/compile.js @@ -746,7 +746,7 @@ test('@mdx-js/mdx: compile', async function (t) { assert.deepEqual( // @ts-expect-error: `_source` is untyped but exists. developmentSourceNode._source, - {fileName: 'path/to/file.js', lineNumber: 1, columnNumber: 1} + {fileName: 'path/to/file.js'} ) } ) @@ -1166,7 +1166,7 @@ test('@mdx-js/mdx: compile (JSX)', async function (t) { '}', 'export default function MDXContent(props = {}) {', ' const {wrapper: MDXLayout} = props.components || ({});', - ' return MDXLayout ? <_createMdxContent {...props} /> : _createMdxContent(props);', + ' return MDXLayout ? <_createMdxContent {...props} /> : <_createMdxContent {...props} />;', '}', '' ].join('\n') @@ -1184,7 +1184,7 @@ test('@mdx-js/mdx: compile (JSX)', async function (t) { '}', 'export default function MDXContent(props = {}) {', ' const {wrapper: MDXLayout} = props.components || ({});', - ' return MDXLayout ? <_createMdxContent {...props} /> : _createMdxContent(props);', + ' return MDXLayout ? <_createMdxContent {...props} /> : <_createMdxContent {...props} />;', '}', '' ].join('\n') @@ -1207,7 +1207,7 @@ test('@mdx-js/mdx: compile (JSX)', async function (t) { '}', 'export default function MDXContent(props = {}) {', ' const {wrapper: MDXLayout} = props.components || ({});', - ' return MDXLayout ? <_createMdxContent {...props} /> : _createMdxContent(props);', + ' return MDXLayout ? <_createMdxContent {...props} /> : <_createMdxContent {...props} />;', '}', 'function _missingMdxReference(id, component) {', ' throw new Error("Expected " + (component ? "component" : "object") + " `" + id + "` to be defined: you likely forgot to import, pass, or provide it.");', @@ -1230,7 +1230,7 @@ test('@mdx-js/mdx: compile (JSX)', async function (t) { '}', 'export default function MDXContent(props = {}) {', ' const {wrapper: MDXLayout} = props.components || ({});', - ' return MDXLayout ? <_createMdxContent {...props} /> : _createMdxContent(props);', + ' return MDXLayout ? <_createMdxContent {...props} /> : <_createMdxContent {...props} />;', '}', '' ].join('\n') @@ -1254,7 +1254,7 @@ test('@mdx-js/mdx: compile (JSX)', async function (t) { '}', 'export default function MDXContent(props = {}) {', ' const {wrapper: MDXLayout} = props.components || ({});', - ' return MDXLayout ? <_createMdxContent {...props} /> : _createMdxContent(props);', + ' return MDXLayout ? <_createMdxContent {...props} /> : <_createMdxContent {...props} />;', '}', '' ].join('\n') @@ -1277,7 +1277,7 @@ test('@mdx-js/mdx: compile (JSX)', async function (t) { '}', 'export default function MDXContent(props = {}) {', ' const {wrapper: MDXLayout} = props.components || ({});', - ' return MDXLayout ? <_createMdxContent {...props} /> : _createMdxContent(props);', + ' return MDXLayout ? <_createMdxContent {...props} /> : <_createMdxContent {...props} />;', '}', '' ].join('\n') @@ -1343,7 +1343,7 @@ test('@mdx-js/mdx: compile (JSX)', async function (t) { ' ..._provideComponents(),', ' ...props.components', ' };', - ' return MDXLayout ? <_createMdxContent {...props} /> : _createMdxContent(props);', + ' return MDXLayout ? <_createMdxContent {...props} /> : <_createMdxContent {...props} />;', '}', '' ].join('\n') diff --git a/packages/mdx/test/syntax.js b/packages/mdx/test/syntax.js index 212b21b77..dc26a14f8 100644 --- a/packages/mdx/test/syntax.js +++ b/packages/mdx/test/syntax.js @@ -662,7 +662,9 @@ test('@mdx-js/mdx: syntax: MDX (JSX)', async function (t) { ' children: _jsx(_createMdxContent, {', ' ...props', ' })', - ' }) : _createMdxContent(props);', + ' }) : _jsx(_createMdxContent, {', + ' ...props', + ' });', '}', '' ].join('\n') @@ -890,7 +892,11 @@ test('@mdx-js/mdx: syntax: MDX (ESM)', async function (t) { ' }, this)', ' }, undefined, false, {', ' fileName: "path/to/file.js"', - ' }, this) : _createMdxContent(props);', + ' }, this) : _jsxDEV(_createMdxContent, {', + ' ...props', + ' }, undefined, false, {', + ' fileName: "path/to/file.js"', + ' }, this);', '}', 'function _missingMdxReference(id, component, place) {', ' throw new Error("Expected " + (component ? "component" : "object") + " `" + id + "` to be defined: you likely forgot to import, pass, or provide it." + (place ? "\\nIt’s referenced in your code at `" + place + "` in `path/to/file.js`" : ""));', @@ -924,7 +930,9 @@ test('@mdx-js/mdx: syntax: MDX (ESM)', async function (t) { ' children: _jsx(_createMdxContent, {', ' ...props', ' })', - ' }) : _createMdxContent(props);', + ' }) : _jsx(_createMdxContent, {', + ' ...props', + ' });', '}', 'return {', ' default: MDXContent', diff --git a/packages/rollup/test/index.js b/packages/rollup/test/index.js index 81d49dc8e..db347f94a 100644 --- a/packages/rollup/test/index.js +++ b/packages/rollup/test/index.js @@ -41,7 +41,7 @@ test('@mdx-js/rollup', async function (t) { // Source map. assert.equal( chunk.map?.mappings, - ';;AAAO,SAAA,OAAA,GAAA;;AAA8B,IAAA,QAAA,EAAA,QAAA;;;;;;;;;AAEnC,IAAA,QAAA,EAAA,CAAA,SAAA,EAAAA,GAAA,CAAA,OAAA,EAAA,EAAA,CAAA,CAAA;;;;;;;;;;;;;;;' + ';;AAAO,SAAA,OAAA,GAAA;;AAA8B,IAAA,QAAA,EAAA,QAAA;;;;;;;;;AAEnC,IAAA,QAAA,EAAA,CAAA,SAAA,EAAAA,GAAA,CAAA,OAAA,EAAA,EAAA,CAAA,CAAA;;;;;;;;;;;;;;;;;' ) await fs.writeFile(jsUrl, chunk.code)