Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MDX optimize option #7151

Merged
merged 8 commits into from
May 25, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/dirty-singers-enjoy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@astrojs/mdx': patch
---

Add `optimize` option for faster builds and rendering
63 changes: 63 additions & 0 deletions packages/integrations/mdx/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ You can configure how your MDX is rendered with the following options:
- [Options inherited from Markdown config](#options-inherited-from-markdown-config)
- [`extendMarkdownConfig`](#extendmarkdownconfig)
- [`recmaPlugins`](#recmaplugins)
- [`optimize`](#optimize)

### Options inherited from Markdown config

Expand Down Expand Up @@ -183,6 +184,68 @@ These are plugins that modify the output [estree](https://github.com/estree/estr

We suggest [using AST Explorer](https://astexplorer.net/) to play with estree outputs, and trying [`estree-util-visit`](https://unifiedjs.com/explore/package/estree-util-visit/) for searching across JavaScript nodes.

### `optimize`
Copy link
Member

Choose a reason for hiding this comment

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

@bluwy Can you also add somthing around line 87 linking to this section?

Also, I'm wondering about the order of these 4 subsections. Optimizing sounds like a pretty important thing, so I might be tempted to put it first if you think it's going to be a no-brainer that most people would enable this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch! I'm thinking of putting this last as a special feature we can point people to turn on if they're having issues. Ideally in the next breaking release when we get good feedback, we can make it the default so the option won't stay here long.


- **Type:** `boolean | { customComponentNames?: string[] }`

This is an optional configuration setting to optimize the MDX output for faster builds and rendering via an internal rehype plugin. This may be useful if you have many MDX content and notice slow builds. However, this option may generate some unescaped HTML, so make sure your site's interactive parts still work correctly after enabling it.

This is disabled by default. To enable MDX optimization, add the following to your MDX integration configuration:

__`astro.config.mjs`__

```js
import { defineConfig } from 'astro/config';
import mdx from '@astrojs/mdx';

export default defineConfig({
integrations: [
mdx({
optimize: true,
})
]
});
```

#### `customComponentNames`

- **Type:** `string[]`

An optional property of `optimize` to prevent the MDX optimizer from handling any [custom components passed to imported MDX content](https://docs.astro.build/en/guides/markdown-content/#custom-components-with-imported-mdx).

You will need to exclude these components from optimization as the optimizer eagerly converts content into a static string, which will break custom components that needs to be dynamically rendered. For example, with the below configuration, the MDX output should be `<Heading>...</Heading>` instead of `"<h1>...</h1>"`.

```astro
---
import { Content, components } from '../content.mdx';
import Heading from '../Heading.astro';
---

<Content components={{...components, h1: Heading }} />
```

Note that if you use `export const components = { ... }` in your MDX file to configure custom components, you don't need to manually configure this option as the optimizer will automatically detect them.

To configure this option, specify an array of HTML element names that are custom components:

__`astro.config.mjs`__

```js
import { defineConfig } from 'astro/config';
import mdx from '@astrojs/mdx';

export default defineConfig({
integrations: [
mdx({
optimize: {
// Prevent the optimizer from handling `h1` elements
customComponentNames: ['h1'],
},
})
]
});
```

## Examples

* The [Astro MDX starter template](https://github.com/withastro/astro/tree/latest/examples/with-mdx) shows how to use MDX files in your Astro project.
Expand Down
1 change: 1 addition & 0 deletions packages/integrations/mdx/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
"estree-util-visit": "^1.2.0",
"github-slugger": "^1.4.0",
"gray-matter": "^4.0.3",
"hast-util-to-html": "^8.0.4",
"kleur": "^4.1.4",
"rehype-raw": "^6.1.1",
"remark-frontmatter": "^4.0.1",
Expand Down
4 changes: 4 additions & 0 deletions packages/integrations/mdx/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { SourceMapGenerator } from 'source-map';
import { VFile } from 'vfile';
import type { Plugin as VitePlugin } from 'vite';
import { getRehypePlugins, getRemarkPlugins, recmaInjectImportMetaEnvPlugin } from './plugins.js';
import type { OptimizeOptions } from './rehype-optimize-static.js';
import { getFileInfo, ignoreStringPlugins, parseFrontmatter } from './utils.js';

export type MdxOptions = Omit<typeof markdownConfigDefaults, 'remarkPlugins' | 'rehypePlugins'> & {
Expand All @@ -22,6 +23,7 @@ export type MdxOptions = Omit<typeof markdownConfigDefaults, 'remarkPlugins' | '
remarkPlugins: PluggableList;
rehypePlugins: PluggableList;
remarkRehype: RemarkRehypeOptions;
optimize: boolean | OptimizeOptions;
};

type SetupHookParams = HookParameters<'astro:config:setup'> & {
Expand Down Expand Up @@ -195,6 +197,7 @@ function markdownConfigToMdxOptions(markdownConfig: typeof markdownConfigDefault
remarkPlugins: ignoreStringPlugins(markdownConfig.remarkPlugins),
rehypePlugins: ignoreStringPlugins(markdownConfig.rehypePlugins),
remarkRehype: (markdownConfig.remarkRehype as any) ?? {},
optimize: false,
};
}

Expand All @@ -215,6 +218,7 @@ function applyDefaultOptions({
remarkPlugins: options.remarkPlugins ?? defaults.remarkPlugins,
rehypePlugins: options.rehypePlugins ?? defaults.rehypePlugins,
shikiConfig: options.shikiConfig ?? defaults.shikiConfig,
optimize: options.optimize ?? defaults.optimize,
};
}

Expand Down
8 changes: 8 additions & 0 deletions packages/integrations/mdx/src/plugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import type { VFile } from 'vfile';
import type { MdxOptions } from './index.js';
import { rehypeInjectHeadingsExport } from './rehype-collect-headings.js';
import rehypeMetaString from './rehype-meta-string.js';
import { rehypeOptimizeStatic } from './rehype-optimize-static.js';
import { remarkImageToComponent } from './remark-images-to-component.js';
import remarkPrism from './remark-prism.js';
import remarkShiki from './remark-shiki.js';
Expand Down Expand Up @@ -145,6 +146,13 @@ export function getRehypePlugins(mdxOptions: MdxOptions): MdxRollupPluginOptions
// computed from `astro.data.frontmatter` in VFile data
rehypeApplyFrontmatterExport,
];

if (mdxOptions.optimize) {
// Convert user `optimize` option to compatible `rehypeOptimizeStatic` option
const options = mdxOptions.optimize === true ? undefined : mdxOptions.optimize;
rehypePlugins.push([rehypeOptimizeStatic, options]);
}

return rehypePlugins;
}

Expand Down
105 changes: 105 additions & 0 deletions packages/integrations/mdx/src/rehype-optimize-static.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
import { visit } from 'estree-util-visit';
import { toHtml } from 'hast-util-to-html';

// accessing untyped hast and mdx types
type Node = any;

export interface OptimizeOptions {
customComponentNames?: string[];
}

const exportConstComponentsRe = /export\s+const\s+components\s*=/;

/**
* For MDX only, collapse static subtrees of the hast into `set:html`. Subtrees
* do not include any MDX elements.
*
* This optimization reduces the JS output as more content are represented as a
* string instead, which also reduces the AST size that Rollup holds in memory.
*/
export function rehypeOptimizeStatic(options?: OptimizeOptions) {
return (tree: any) => {
// A set of non-static components to avoid collapsing when walking the tree
// as they need to be preserved as JSX to be rendered dynamically.
const customComponentNames = new Set<string>(options?.customComponentNames);

// Find `export const components = { ... }` and get it's object's keys to be
// populated into `customComponentNames`. This configuration is used to render
// some HTML elements as custom components, and we also want to avoid collapsing them.
for (const child of tree.children) {
if (child.type === 'mdxjsEsm' && exportConstComponentsRe.test(child.value)) {
// Try to loosely get the object property nodes
const objectPropertyNodes = child.data.estree.body[0]?.declarations?.[0]?.init?.properties;
if (objectPropertyNodes) {
for (const objectPropertyNode of objectPropertyNodes) {
const componentName = objectPropertyNode.key?.name ?? objectPropertyNode.key?.value;
if (componentName) {
customComponentNames.add(componentName);
}
}
}
}
}

// All possible elements that could be the root of a subtree
const allPossibleElements = new Set<Node>();
// The current collapsible element stack while traversing the tree
const elementStack: Node[] = [];

visit(tree, {
enter(node) {
// @ts-expect-error read tagName naively
const isCustomComponent = node.tagName && customComponentNames.has(node.tagName);
// For nodes that can't be optimized, eliminate all elements in the
// `elementStack` from the `allPossibleElements` set.
if (node.type.startsWith('mdx') || isCustomComponent) {
for (const el of elementStack) {
allPossibleElements.delete(el);
}
// Micro-optimization: While this destroys the meaning of an element
// stack for this node, things will still work but we won't repeatedly
// run the above for other nodes anymore. If this is confusing, you can
// comment out the code below when reading.
elementStack.length = 0;
Copy link
Member

Choose a reason for hiding this comment

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

nit: What confuses me is that we drain the elementStack inside an if block before another if block. Logically, we can enter both if, which means we can drain the stack, push nodes, and then do it again.

The comment fails to address that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah the idea is that we can and want to enter both if block, because if this node is non-optimizable, that doesn't mean we can't optimize the children into this node.

For example:

<Foo>
  <Bar>
    # this is markdown
  </Bar>
</Foo>

Can be turned into:

<Foo>
  <Bar set:html="<h1>this is markdown</h1>"></Bar>
</Foo>

So Bar here is something we want to track because in the leave hook later, we can add it to allPossibleElements.


I don't think it's easy to explain this as a comment because this optimization only comes clear when you read it last after understanding the (unfortunately complex 😅) algorithm.

Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't mind having a README.md next to this file that explains a bit about what we do :) it's not always easy to explain what's on our mind in a few comments! I completely understand

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that's a good idea. I'll note a followup PR to document this so the docs can be reviewed separately.

}
// For possible subtree root nodes, record them in `elementStack` and
// `allPossibleElements` to be used in the "leave" hook below.
if (node.type === 'element' || node.type === 'mdxJsxFlowElement') {
elementStack.push(node);
allPossibleElements.add(node);
}
},
leave(node, _, __, parents) {
// Do the reverse of the if condition above, popping the `elementStack`,
// and consolidating `allPossibleElements` as a subtree root.
if (node.type === 'element' || node.type === 'mdxJsxFlowElement') {
elementStack.pop();
// Many possible elements could be part of a subtree, in order to find
// the root, we check the parent of the element we're popping. If the
// parent exists in `allPossibleElements`, then we're definitely not
// the root, so remove ourselves. This will work retroactively as we
// climb back up the tree.
const parent = parents[parents.length - 1];
if (allPossibleElements.has(parent)) {
allPossibleElements.delete(node);
}
}
},
});

// For all possible subtree roots, collapse them into `set:html` and
// strip of their children
for (const el of allPossibleElements) {
if (el.type === 'mdxJsxFlowElement') {
el.attributes.push({
type: 'mdxJsxAttribute',
name: 'set:html',
value: toHtml(el.children),
});
} else {
el.properties['set:html'] = toHtml(el.children);
}
el.children = [];
}
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import mdx from '@astrojs/mdx';

export default {
integrations: [mdx({
optimize: {
customComponentNames: ['strong']
}
})]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "@test/mdx-optimize",
"private": true,
"dependencies": {
"@astrojs/mdx": "workspace:*",
"astro": "workspace:*"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<blockquote {...Astro.props} class="custom-blockquote">
<slot />
</blockquote>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<strong {...Astro.props} class="custom-strong">
<slot />
</strong>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
I once heard a very **inspirational** quote:

> I like pancakes
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
import { Content, components } from './index.mdx'
import Strong from '../components/Strong.astro'
---

<!DOCTYPE html>
<html>
<head>
<title>Import MDX component</title>
</head>
<body>
<h1>Astro page</h1>
<Content components={{ ...components, strong: Strong }} />
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import Blockquote from '../components/Blockquote.astro'

export const components = {
blockquote: Blockquote
}

# MDX page

I once heard a very inspirational quote:

> I like pancakes

```js
const pancakes = 'yummy'
```
47 changes: 47 additions & 0 deletions packages/integrations/mdx/test/mdx-optimize.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { expect } from 'chai';
import { parseHTML } from 'linkedom';
import { loadFixture } from '../../../astro/test/test-utils.js';

const FIXTURE_ROOT = new URL('./fixtures/mdx-optimize/', import.meta.url);

describe('MDX optimize', () => {
let fixture;
before(async () => {
fixture = await loadFixture({
root: FIXTURE_ROOT,
});
await fixture.build();
});

it('renders an MDX page fine', async () => {
const html = await fixture.readFile('/index.html');
const { document } = parseHTML(html);

expect(document.querySelector('h1').textContent).include('MDX page');
expect(document.querySelector('p').textContent).include(
'I once heard a very inspirational quote:'
);

const blockquote = document.querySelector('blockquote.custom-blockquote');
expect(blockquote).to.not.be.null;
expect(blockquote.textContent).to.include('I like pancakes');

const code = document.querySelector('pre.astro-code');
expect(code).to.not.be.null;
expect(code.textContent).to.include(`const pancakes = 'yummy'`);
});

it('renders an Astro page that imports MDX fine', async () => {
const html = await fixture.readFile('/import/index.html');
const { document } = parseHTML(html);

expect(document.querySelector('h1').textContent).include('Astro page');
expect(document.querySelector('p').textContent).include(
'I once heard a very inspirational quote:'
);

const blockquote = document.querySelector('blockquote.custom-blockquote');
expect(blockquote).to.not.be.null;
expect(blockquote.textContent).to.include('I like pancakes');
});
});
12 changes: 12 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.