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 2 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
bluwy marked this conversation as resolved.
Show resolved Hide resolved
---

Add `optimize` option for faster builds and rendering
36 changes: 36 additions & 0 deletions packages/integrations/mdx/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,42 @@ 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.


Whether to optimize the MDX output for faster builds and rendering. An internal rehype plugin will be injected last to transform static parts of the `hast` into [`set:html` properties](https://docs.astro.build/en/reference/directives-reference/#sethtml). As this modifies the intermediate `estree` and leaves some generated HTML to be un-escaped, the option is disabled by default.
bluwy marked this conversation as resolved.
Show resolved Hide resolved

As the optimizer works within individual MDX files, if you're [passing custom components to an imported MDX content](https://docs.astro.build/en/guides/markdown-content/#custom-components-with-imported-mdx), you'll need to configure `customComponentNames` to prevent the optimizer from handling the custom components:
bluwy marked this conversation as resolved.
Show resolved Hide resolved

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

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

__`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'],
},
})
]
});
```

> **Note**
> If you're using `export const components = { ... }` in your MDX file, the optimizer will automatically detect and add them to `customComponentNames` so you don't have to manually configure it.
bluwy marked this conversation as resolved.
Show resolved Hide resolved

## 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
7 changes: 7 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,12 @@ export function getRehypePlugins(mdxOptions: MdxOptions): MdxRollupPluginOptions
// computed from `astro.data.frontmatter` in VFile data
rehypeApplyFrontmatterExport,
];

if (mdxOptions.optimize) {
const options = mdxOptions.optimize === true ? undefined : mdxOptions.optimize;
bluwy marked this conversation as resolved.
Show resolved Hide resolved
rehypePlugins.push([rehypeOptimizeStatic, options]);
}

return rehypePlugins;
}

Expand Down
97 changes: 97 additions & 0 deletions packages/integrations/mdx/src/rehype-optimize-static.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
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) => {
// Read `export const components` to make sure the components are not optimized.
const customComponentNames = new Set<string>(options?.customComponentNames);
for (const child of tree.children) {
bluwy marked this conversation as resolved.
Show resolved Hide resolved
if (child.type === 'mdxjsEsm' && exportConstComponentsRe.test(child.value)) {
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);
}
bluwy marked this conversation as resolved.
Show resolved Hide resolved
// 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
if (node.type === 'element' || node.type === 'mdxJsxFlowElement') {
elementStack.push(node);
allPossibleElements.add(node);
}
},
leave(node, _, __, parents) {
// Similar as above, but pop the `elementStack`
bluwy marked this conversation as resolved.
Show resolved Hide resolved
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.