Skip to content

Commit

Permalink
Fix some performance by improving vdom diffing
Browse files Browse the repository at this point in the history 
Closes GH-2029.
Closes GH-2062.

Reviewed-by: Titus Wormer <tituswormer@gmail.com>
Reviewed-by: Christian Murphy <christian.murphy.42@gmail.com>
  • Loading branch information
@0phoff
0phoff committed Jun 17, 2022
1 parent ac44dc7 commit 7bcd705
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 94 deletions.
94 changes: 53 additions & 41 deletions packages/mdx/lib/plugin/recma-document.js
View file
Expand Up @@ -231,7 +231,7 @@ export function recmaDocument(options = {}) {
child.expression.type === 'JSXElement')
) {
content = true
replacement.push(createMdxContent(child.expression))
replacement.push(...createMdxContent(child.expression, Boolean(layout)))
// The following catch-all branch is because plugins might’ve added
// other things.
// Normally, we only have import/export/jsx, but just add whatever’s
Expand All @@ -244,7 +244,7 @@ export function recmaDocument(options = {}) {

// If there was no JSX content at all, add an empty function.
if (!content) {
replacement.push(createMdxContent())
replacement.push(...createMdxContent(undefined, Boolean(layout)))
}

exportedIdentifiers.push(['MDXContent', 'default'])
Expand Down Expand Up @@ -481,9 +481,10 @@ export function recmaDocument(options = {}) {

/**
* @param {Expression} [content]
* @returns {FunctionDeclaration}
* @param {boolean} [hasInternalLayout]
* @returns {FunctionDeclaration[]}
*/
function createMdxContent(content) {
function createMdxContent(content, hasInternalLayout) {
/** @type {JSXElement} */
const element = {
type: 'JSXElement',
Expand All @@ -508,7 +509,12 @@ export function recmaDocument(options = {}) {
openingElement: {
type: 'JSXOpeningElement',
name: {type: 'JSXIdentifier', name: '_createMdxContent'},
attributes: [],
attributes: [
{
type: 'JSXSpreadAttribute',
argument: {type: 'Identifier', name: 'props'}
}
],
selfClosing: true
},
closingElement: null,
Expand All @@ -518,7 +524,21 @@ export function recmaDocument(options = {}) {
}

// @ts-expect-error: JSXElements are expressions.
const consequent = /** @type {Expression} */ (element)
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
}
}
}

let argument = content || {type: 'Literal', value: null}

Expand All @@ -535,44 +555,36 @@ export function recmaDocument(options = {}) {
argument = argument.children[0]
}

return {
type: 'FunctionDeclaration',
id: {type: 'Identifier', name: 'MDXContent'},
params: [
{
type: 'AssignmentPattern',
left: {type: 'Identifier', name: 'props'},
right: {type: 'ObjectExpression', properties: []}
return [
{
type: 'FunctionDeclaration',
id: {type: 'Identifier', name: '_createMdxContent'},
params: [{type: 'Identifier', name: 'props'}],
body: {
type: 'BlockStatement',
body: [{type: 'ReturnStatement', argument}]
}
],
body: {
type: 'BlockStatement',
body: [
{
type: 'ReturnStatement',
argument: {
type: 'ConditionalExpression',
test: {type: 'Identifier', name: 'MDXLayout'},
consequent,
alternate: {
type: 'CallExpression',
callee: {type: 'Identifier', name: '_createMdxContent'},
arguments: [],
optional: false
}
}
},
},
{
type: 'FunctionDeclaration',
id: {type: 'Identifier', name: 'MDXContent'},
params: [
{
type: 'FunctionDeclaration',
id: {type: 'Identifier', name: '_createMdxContent'},
params: [],
body: {
type: 'BlockStatement',
body: [{type: 'ReturnStatement', argument}]
}
type: 'AssignmentPattern',
left: {type: 'Identifier', name: 'props'},
right: {type: 'ObjectExpression', properties: []}
}
]
],
body: {
type: 'BlockStatement',
body: [
{
type: 'ReturnStatement',
argument: result
}
]
}
}
}
]
}
}
30 changes: 15 additions & 15 deletions packages/mdx/lib/plugin/recma-jsx-rewrite.js
View file
Expand Up @@ -76,6 +76,10 @@ export function recmaJsxRewrite(options = {}) {
walk(tree, {
enter(_node) {
const node = /** @type {Node} */ (_node)
const newScope = /** @type {Scope|undefined} */ (
// @ts-expect-error: periscopic doesn’t support JSX.
scopeInfo.map.get(node)
)

if (
node.type === 'FunctionDeclaration' ||
Expand All @@ -89,30 +93,26 @@ export function recmaJsxRewrite(options = {}) {
references: {},
node
})
}

let fnScope = fnStack[0]
// MDXContent only ever contains MDXLayout
if (
isNamedFunction(node, 'MDXContent') &&
newScope &&
!inScope(newScope, 'MDXLayout')
) {
fnStack[0].components.push('MDXLayout')
}
}

const fnScope = fnStack[0]
if (
!fnScope ||
(!isNamedFunction(fnScope.node, 'MDXContent') &&
(!isNamedFunction(fnScope.node, '_createMdxContent') &&
!providerImportSource)
) {
return
}

if (
fnStack[1] &&
isNamedFunction(fnStack[1].node, '_createMdxContent')
) {
fnScope = fnStack[1]
}

const newScope = /** @type {Scope|undefined} */ (
// @ts-expect-error: periscopic doesn’t support JSX.
scopeInfo.map.get(node)
)

if (newScope) {
newScope.node = node
currentScope = newScope
Expand Down
101 changes: 64 additions & 37 deletions packages/mdx/test/compile.js
View file
Expand Up @@ -778,16 +778,16 @@ test('jsx', async () => {
String(compileSync('*a*', {jsx: true})),
[
'/*@jsxRuntime automatic @jsxImportSource react*/',
'function _createMdxContent(props) {',
' const _components = Object.assign({',
' p: "p",',
' em: "em"',
' }, props.components);',
' return <_components.p><_components.em>{"a"}</_components.em></_components.p>;',
'}',
'function MDXContent(props = {}) {',
' const {wrapper: MDXLayout} = props.components || ({});',
' return MDXLayout ? <MDXLayout {...props}><_createMdxContent /></MDXLayout> : _createMdxContent();',
' function _createMdxContent() {',
' const _components = Object.assign({',
' p: "p",',
' em: "em"',
' }, props.components);',
' return <_components.p><_components.em>{"a"}</_components.em></_components.p>;',
' }',
' return MDXLayout ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout> : _createMdxContent(props);',
'}',
'export default MDXContent;',
''
Expand All @@ -799,12 +799,12 @@ test('jsx', async () => {
String(compileSync('<a {...b} c d="1" e={1} />', {jsx: true})),
[
'/*@jsxRuntime automatic @jsxImportSource react*/',
'function _createMdxContent(props) {',
' return <a {...b} c d="1" e={1} />;',
'}',
'function MDXContent(props = {}) {',
' const {wrapper: MDXLayout} = props.components || ({});',
' return MDXLayout ? <MDXLayout {...props}><_createMdxContent /></MDXLayout> : _createMdxContent();',
' function _createMdxContent() {',
' return <a {...b} c d="1" e={1} />;',
' }',
' return MDXLayout ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout> : _createMdxContent(props);',
'}',
'export default MDXContent;',
''
Expand All @@ -816,15 +816,15 @@ test('jsx', async () => {
String(compileSync('<><a:b /><c.d/></>', {jsx: true})),
[
'/*@jsxRuntime automatic @jsxImportSource react*/',
'function _createMdxContent(props) {',
' const {c} = props.components || ({});',
' if (!c) _missingMdxReference("c", false);',
' if (!c.d) _missingMdxReference("c.d", true);',
' return <><><a:b /><c.d /></></>;',
'}',
'function MDXContent(props = {}) {',
' const {wrapper: MDXLayout} = props.components || ({});',
' return MDXLayout ? <MDXLayout {...props}><_createMdxContent /></MDXLayout> : _createMdxContent();',
' function _createMdxContent() {',
' const {c} = props.components || ({});',
' if (!c) _missingMdxReference("c", false);',
' if (!c.d) _missingMdxReference("c.d", true);',
' return <><><a:b /><c.d /></></>;',
' }',
' return MDXLayout ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout> : _createMdxContent(props);',
'}',
'export default MDXContent;',
'function _missingMdxReference(id, component) {',
Expand All @@ -840,12 +840,12 @@ test('jsx', async () => {
[
'/*@jsxRuntime automatic @jsxImportSource react*/',
'/*1*/',
'function _createMdxContent(props) {',
' return <><>{"a "}{}{" b"}</></>;',
'}',
'function MDXContent(props = {}) {',
' const {wrapper: MDXLayout} = props.components || ({});',
' return MDXLayout ? <MDXLayout {...props}><_createMdxContent /></MDXLayout> : _createMdxContent();',
' function _createMdxContent() {',
' return <><>{"a "}{}{" b"}</></>;',
' }',
' return MDXLayout ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout> : _createMdxContent(props);',
'}',
'export default MDXContent;',
''
Expand All @@ -857,15 +857,15 @@ test('jsx', async () => {
String(compileSync('{<a-b></a-b>}', {jsx: true})),
[
'/*@jsxRuntime automatic @jsxImportSource react*/',
'function _createMdxContent(props) {',
' const _components = Object.assign({',
' "a-b": "a-b"',
' }, props.components);',
' return <>{<_components.a-b></_components.a-b>}</>;',
'}',
'function MDXContent(props = {}) {',
' const {wrapper: MDXLayout} = props.components || ({});',
' return MDXLayout ? <MDXLayout {...props}><_createMdxContent /></MDXLayout> : _createMdxContent();',
' function _createMdxContent() {',
' const _components = Object.assign({',
' "a-b": "a-b"',
' }, props.components);',
' return <>{<_components.a-b></_components.a-b>}</>;',
' }',
' return MDXLayout ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout> : _createMdxContent(props);',
'}',
'export default MDXContent;',
''
Expand All @@ -877,22 +877,49 @@ test('jsx', async () => {
String(compileSync('Hello {props.name}', {jsx: true})),
[
'/*@jsxRuntime automatic @jsxImportSource react*/',
'function _createMdxContent(props) {',
' const _components = Object.assign({',
' p: "p"',
' }, props.components);',
' return <_components.p>{"Hello "}{props.name}</_components.p>;',
'}',
'function MDXContent(props = {}) {',
' const {wrapper: MDXLayout} = props.components || ({});',
' return MDXLayout ? <MDXLayout {...props}><_createMdxContent /></MDXLayout> : _createMdxContent();',
' function _createMdxContent() {',
' const _components = Object.assign({',
' p: "p"',
' }, props.components);',
' return <_components.p>{"Hello "}{props.name}</_components.p>;',
' }',
' return MDXLayout ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout> : _createMdxContent(props);',
'}',
'export default MDXContent;',
''
].join('\n'),
'should allow using props'
)

assert.equal(
String(
compileSync(
'export default function Layout({components, ...props}) { return <section {...props} /> }\n\na',
{jsx: true}
)
),
[
'/*@jsxRuntime automatic @jsxImportSource react*/',
'const MDXLayout = function Layout({components, ...props}) {',
' return <section {...props} />;',
'};',
'function _createMdxContent(props) {',
' const _components = Object.assign({',
' p: "p"',
' }, props.components);',
' return <_components.p>{"a"}</_components.p>;',
'}',
'function MDXContent(props = {}) {',
' return <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout>;',
'}',
'export default MDXContent;',
''
].join('\n'),
'should not have a conditional expression for MDXLayout when there is an internal layout'
)

assert.match(
String(compileSync("{<w x='y \" z' />}", {jsx: true})),
/x="y &quot; z"/,
Expand Down
2 changes: 1 addition & 1 deletion packages/rollup/test/index.test.js
View file
Expand Up @@ -34,7 +34,7 @@ test('@mdx-js/rollup', async () => {

assert.equal(
output[0].map ? output[0].map.mappings : undefined,
';;;MAAaA,OAAU,GAAA,MAAAC,GAAA,CAAAC,QAAA,EAAA;AAAQ,EAAA,QAAA,EAAA,QAAA;;;;;;;;;;;;AAE7B,MAAA,QAAA,EAAA,CAAA,SAAA,EAAAD,GAAA,CAAA,OAAA,EAAA,EAAA,CAAA,CAAA;;;;;;;',
';;;MAAaA,OAAU,GAAA,MAAAC,GAAA,CAAAC,QAAA,EAAA;AAAQ,EAAA,QAAA,EAAA,QAAA;;;;;;;AAE7B,IAAA,QAAA,EAAA,CAAA,SAAA,EAAAD,GAAA,CAAA,OAAA,EAAA,EAAA,CAAA,CAAA;;;;;;;;;;;;',
'should add a source map'
)

Expand Down

1 comment on commit 7bcd705

@vercel
Copy link

@vercel vercel bot commented on 7bcd705 Jun 17, 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 – ./

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

Please sign in to comment.