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

Tag MDX component for faster checks when rendering #10864

Merged
merged 1 commit into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from all 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/chilly-items-help.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"astro": patch
---

Improves the error message when failed to render MDX components
5 changes: 5 additions & 0 deletions .changeset/tame-avocados-relax.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@astrojs/mdx": patch
---

Tags the MDX component export for quicker component checks while rendering
38 changes: 24 additions & 14 deletions packages/astro/src/jsx/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { renderJSX } from '../runtime/server/jsx.js';

const slotName = (str: string) => str.trim().replace(/[-_]([a-z])/g, (_, w) => w.toUpperCase());

// NOTE: In practice, MDX components are always tagged with `__astro_tag_component__`, so the right renderer
// is used directly, and this check is not often used to return true.
export async function check(
Component: any,
props: any,
Expand All @@ -19,18 +21,7 @@ export async function check(
const result = await Component({ ...props, ...slots, children });
return result[AstroJSX];
} catch (e) {
const error = e as Error;
// if the exception is from an mdx component
// throw an error
if (Component[Symbol.for('mdx-component')]) {
throw new AstroError({
message: error.message,
title: error.name,
hint: `This issue often occurs when your MDX component encounters runtime errors.`,
name: error.name,
stack: error.stack,
});
}
throwEnhancedErrorIfMdxComponent(e as Error, Component);
}
return false;
}
Expand All @@ -48,8 +39,27 @@ export async function renderToStaticMarkup(
}

const { result } = this;
const html = await renderJSX(result, jsx(Component, { ...props, ...slots, children }));
return { html };
try {
const html = await renderJSX(result, jsx(Component, { ...props, ...slots, children }));
return { html };
} catch (e) {
throwEnhancedErrorIfMdxComponent(e as Error, Component);
throw e;
}
}

function throwEnhancedErrorIfMdxComponent(error: Error, Component: any) {
// if the exception is from an mdx component
// throw an error
if (Component[Symbol.for('mdx-component')]) {
ematipico marked this conversation as resolved.
Show resolved Hide resolved
throw new AstroError({
message: error.message,
title: error.name,
hint: `This issue often occurs when your MDX component encounters runtime errors.`,
name: error.name,
stack: error.stack,
});
}
}

export default {
Expand Down
65 changes: 48 additions & 17 deletions packages/integrations/mdx/src/vite-plugin-mdx-postprocess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,14 @@ import {
} from './remark-images-to-component.js';
import { type FileInfo, getFileInfo } from './utils.js';

const fragmentImportRegex = /[\s,{]Fragment[\s,}]/;
const astroTagComponentImportRegex = /[\s,{]__astro_tag_component__[\s,}]/;

// These transforms must happen *after* JSX runtime transformations
export function vitePluginMdxPostprocess(astroConfig: AstroConfig): Plugin {
return {
name: '@astrojs/mdx-postprocess',
transform(code, id) {
transform(code, id, opts) {
if (!id.endsWith('.mdx')) return;

const fileInfo = getFileInfo(id, astroConfig);
Expand All @@ -22,7 +25,7 @@ export function vitePluginMdxPostprocess(astroConfig: AstroConfig): Plugin {
code = injectFragmentImport(code, imports);
code = injectMetadataExports(code, exports, fileInfo);
code = transformContentExport(code, exports);
code = annotateContentExport(code, id);
code = annotateContentExport(code, id, !!opts?.ssr, imports);

// The code transformations above are append-only, so the line/column mappings are the same
// and we can omit the sourcemap for performance.
Expand All @@ -31,23 +34,12 @@ export function vitePluginMdxPostprocess(astroConfig: AstroConfig): Plugin {
};
}

const fragmentImportRegex = /[\s,{](?:Fragment,|Fragment\s*\})/;

/**
* Inject `Fragment` identifier import if not already present. It should already be injected,
* but check just to be safe.
*
* TODO: Double-check if we no longer need this function.
* Inject `Fragment` identifier import if not already present.
*/
function injectFragmentImport(code: string, imports: readonly ImportSpecifier[]) {
const importsFromJSXRuntime = imports
.filter(({ n }) => n === 'astro/jsx-runtime')
.map(({ ss, se }) => code.substring(ss, se));
const hasFragmentImport = importsFromJSXRuntime.some((statement) =>
fragmentImportRegex.test(statement)
);
if (!hasFragmentImport) {
code = `import { Fragment } from "astro/jsx-runtime"\n` + code;
if (!isSpecifierImported(code, imports, fragmentImportRegex, 'astro/jsx-runtime')) {
code += `\nimport { Fragment } from 'astro/jsx-runtime';`;
Comment on lines -43 to +42
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 updated the code = to code +=. This causes the import statement to append the code, but ESM hoists the import specifiers, so you can actually use Fragment beforehand.

The reason I append is to avoid generating a sourcemap as we don't alter existing code positions.

}
return code;
}
Expand Down Expand Up @@ -103,13 +95,52 @@ export default Content;`;
/**
* Add properties to the `Content` export.
*/
function annotateContentExport(code: string, id: string) {
function annotateContentExport(
code: string,
id: string,
ssr: boolean,
imports: readonly ImportSpecifier[]
) {
// Mark `Content` as MDX component
code += `\nContent[Symbol.for('mdx-component')] = true`;
// Ensure styles and scripts are injected into a `<head>` when a layout is not applied
code += `\nContent[Symbol.for('astro.needsHeadRendering')] = !Boolean(frontmatter.layout);`;
// Assign the `moduleId` metadata to `Content`
code += `\nContent.moduleId = ${JSON.stringify(id)};`;

// Tag the `Content` export as "astro:jsx" so it's quicker to identify how to render this component
if (ssr) {
if (
!isSpecifierImported(
code,
imports,
astroTagComponentImportRegex,
'astro/runtime/server/index.js'
)
) {
code += `\nimport { __astro_tag_component__ } from 'astro/runtime/server/index.js';`;
ematipico marked this conversation as resolved.
Show resolved Hide resolved
}
code += `\n__astro_tag_component__(Content, 'astro:jsx');`;
Comment on lines +111 to +123
Copy link
Member Author

Choose a reason for hiding this comment

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

This mimics tag.ts but specific to the MDX component so it's much simpler. We only need to tag the default exported Content component. I don't think there will be other exports that are MDX components.

}

return code;
}

/**
* Check whether the `specifierRegex` matches for an import of `source` in the `code`.
*/
function isSpecifierImported(
code: string,
imports: readonly ImportSpecifier[],
specifierRegex: RegExp,
source: string
) {
for (const imp of imports) {
if (imp.n !== source) continue;

const importStatement = code.slice(imp.ss, imp.se);
if (specifierRegex.test(importStatement)) return true;
}

return false;
}
Loading