Skip to content

Commit

Permalink
Always treat _createMdxContent as a JSX component
Browse files Browse the repository at this point in the history
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
  • Loading branch information
remcohaszing committed Feb 25, 2024
1 parent 8f754f7 commit 225b9ce
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 46 deletions.
2 changes: 1 addition & 1 deletion docs/docs/using-mdx.mdx
Expand Up @@ -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})
}
```

Expand Down
2 changes: 1 addition & 1 deletion docs/guides/injecting-components.mdx
Expand Up @@ -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
Expand Down
54 changes: 26 additions & 28 deletions packages/mdx/lib/plugin/recma-document.js
Expand Up @@ -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',
Expand All @@ -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()
}
}

Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions packages/mdx/readme.md
Expand Up @@ -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})
}
```

Expand Down Expand Up @@ -678,7 +678,7 @@ Configuration for `createProcessor` (TypeScript type).
- children: _jsx(_createMdxContent, props)
- })
+ ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout>
: _createMdxContent(props)
: _jsx(_createMdxContent, {...props})
}
}
```
Expand Down Expand Up @@ -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})
```

</details>
Expand Down
16 changes: 8 additions & 8 deletions packages/mdx/test/compile.js
Expand Up @@ -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'}
)
}
)
Expand Down Expand Up @@ -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 ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout> : _createMdxContent(props);',
' return MDXLayout ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout> : <_createMdxContent {...props} />;',
'}',
''
].join('\n')
Expand All @@ -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 ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout> : _createMdxContent(props);',
' return MDXLayout ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout> : <_createMdxContent {...props} />;',
'}',
''
].join('\n')
Expand All @@ -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 ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout> : _createMdxContent(props);',
' return MDXLayout ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout> : <_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.");',
Expand All @@ -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 ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout> : _createMdxContent(props);',
' return MDXLayout ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout> : <_createMdxContent {...props} />;',
'}',
''
].join('\n')
Expand All @@ -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 ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout> : _createMdxContent(props);',
' return MDXLayout ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout> : <_createMdxContent {...props} />;',
'}',
''
].join('\n')
Expand All @@ -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 ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout> : _createMdxContent(props);',
' return MDXLayout ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout> : <_createMdxContent {...props} />;',
'}',
''
].join('\n')
Expand Down Expand Up @@ -1343,7 +1343,7 @@ test('@mdx-js/mdx: compile (JSX)', async function (t) {
' ..._provideComponents(),',
' ...props.components',
' };',
' return MDXLayout ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout> : _createMdxContent(props);',
' return MDXLayout ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout> : <_createMdxContent {...props} />;',
'}',
''
].join('\n')
Expand Down
14 changes: 11 additions & 3 deletions packages/mdx/test/syntax.js
Expand Up @@ -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')
Expand Down Expand Up @@ -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`" : ""));',
Expand Down Expand Up @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion packages/rollup/test/index.js
Expand Up @@ -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)
Expand Down

0 comments on commit 225b9ce

Please sign in to comment.