From cf2bba1e4a32ff7d424cc1c4954d6328167af8d7 Mon Sep 17 00:00:00 2001 From: Nate Moore Date: Fri, 30 Sep 2022 17:13:44 -0500 Subject: [PATCH] P5: fix MDX memory leak (#4939) * fix(astro): tag jsx vnodes with renderer so errors are properly handled * chore: fix missing package in test Co-authored-by: Nate Moore --- .changeset/tall-files-smash.md | 5 ++++ packages/astro/src/jsx-runtime/index.ts | 4 ++- packages/astro/src/runtime/server/jsx.ts | 4 +++ .../mdx-infinite-loop/astro.config.ts | 6 ++++ .../fixtures/mdx-infinite-loop/package.json | 10 +++++++ .../mdx-infinite-loop/src/components/Test.js | 3 ++ .../mdx-infinite-loop/src/pages/doc.mdx | 6 ++++ .../mdx-infinite-loop/src/pages/index.astro | 5 ++++ .../mdx/test/mdx-infinite-loop.test.js | 30 +++++++++++++++++++ pnpm-lock.yaml | 12 ++++++++ 10 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 .changeset/tall-files-smash.md create mode 100644 packages/integrations/mdx/test/fixtures/mdx-infinite-loop/astro.config.ts create mode 100644 packages/integrations/mdx/test/fixtures/mdx-infinite-loop/package.json create mode 100644 packages/integrations/mdx/test/fixtures/mdx-infinite-loop/src/components/Test.js create mode 100644 packages/integrations/mdx/test/fixtures/mdx-infinite-loop/src/pages/doc.mdx create mode 100644 packages/integrations/mdx/test/fixtures/mdx-infinite-loop/src/pages/index.astro create mode 100644 packages/integrations/mdx/test/mdx-infinite-loop.test.js diff --git a/.changeset/tall-files-smash.md b/.changeset/tall-files-smash.md new file mode 100644 index 0000000000..537fe49f5d --- /dev/null +++ b/.changeset/tall-files-smash.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fix MDX error handling, preventing a memory leak diff --git a/packages/astro/src/jsx-runtime/index.ts b/packages/astro/src/jsx-runtime/index.ts index 20630a82a0..f0a476f827 100644 --- a/packages/astro/src/jsx-runtime/index.ts +++ b/packages/astro/src/jsx-runtime/index.ts @@ -1,9 +1,10 @@ -import { Fragment, markHTMLString } from '../runtime/server/index.js'; +import { Renderer, Fragment, markHTMLString } from '../runtime/server/index.js'; const AstroJSX = 'astro:jsx'; const Empty = Symbol('empty'); export interface AstroVNode { + [Renderer]: string; [AstroJSX]: boolean; type: string | ((...args: any) => any); props: Record; @@ -74,6 +75,7 @@ function transformSetDirectives(vnode: AstroVNode) { function createVNode(type: any, props: Record) { const vnode: AstroVNode = { + [Renderer]: 'astro:jsx', [AstroJSX]: true, type, props: props ?? {}, diff --git a/packages/astro/src/runtime/server/jsx.ts b/packages/astro/src/runtime/server/jsx.ts index 3e5816f28d..6ee12a406c 100644 --- a/packages/astro/src/runtime/server/jsx.ts +++ b/packages/astro/src/runtime/server/jsx.ts @@ -38,6 +38,10 @@ export async function renderJSX(result: SSRResult, vnode: any): Promise { } if (isVNode(vnode)) { switch (true) { + case !vnode.type: { + throw new Error(`Unable to render ${result._metadata.pathname} because it contains an undefined Component! +Did you forget to import the component or is it possible there is a typo?`) + } case (vnode.type as any) === Symbol.for('astro:fragment'): return renderJSX(result, vnode.props.children); case (vnode.type as any).isAstroComponentFactory: { diff --git a/packages/integrations/mdx/test/fixtures/mdx-infinite-loop/astro.config.ts b/packages/integrations/mdx/test/fixtures/mdx-infinite-loop/astro.config.ts new file mode 100644 index 0000000000..75a2b5f3a4 --- /dev/null +++ b/packages/integrations/mdx/test/fixtures/mdx-infinite-loop/astro.config.ts @@ -0,0 +1,6 @@ +import mdx from '@astrojs/mdx'; +import preact from '@astrojs/preact'; + +export default { + integrations: [mdx(), preact()] +} diff --git a/packages/integrations/mdx/test/fixtures/mdx-infinite-loop/package.json b/packages/integrations/mdx/test/fixtures/mdx-infinite-loop/package.json new file mode 100644 index 0000000000..5d1ab56321 --- /dev/null +++ b/packages/integrations/mdx/test/fixtures/mdx-infinite-loop/package.json @@ -0,0 +1,10 @@ +{ + "name": "@test/mdx-infinite-loop", + "type": "module", + "dependencies": { + "@astrojs/mdx": "workspace:*", + "@astrojs/preact": "workspace:*", + "preact": "^10.7.3", + "astro": "workspace:*" + } +} diff --git a/packages/integrations/mdx/test/fixtures/mdx-infinite-loop/src/components/Test.js b/packages/integrations/mdx/test/fixtures/mdx-infinite-loop/src/components/Test.js new file mode 100644 index 0000000000..831fb327c7 --- /dev/null +++ b/packages/integrations/mdx/test/fixtures/mdx-infinite-loop/src/components/Test.js @@ -0,0 +1,3 @@ +export default function () { + return 'Hello world' +} diff --git a/packages/integrations/mdx/test/fixtures/mdx-infinite-loop/src/pages/doc.mdx b/packages/integrations/mdx/test/fixtures/mdx-infinite-loop/src/pages/doc.mdx new file mode 100644 index 0000000000..8b8dc5e07e --- /dev/null +++ b/packages/integrations/mdx/test/fixtures/mdx-infinite-loop/src/pages/doc.mdx @@ -0,0 +1,6 @@ +import Test, { Missing } from '../components/Test'; + +# Hello page! + + + diff --git a/packages/integrations/mdx/test/fixtures/mdx-infinite-loop/src/pages/index.astro b/packages/integrations/mdx/test/fixtures/mdx-infinite-loop/src/pages/index.astro new file mode 100644 index 0000000000..11f7af3859 --- /dev/null +++ b/packages/integrations/mdx/test/fixtures/mdx-infinite-loop/src/pages/index.astro @@ -0,0 +1,5 @@ +--- +const files = await Astro.glob('./**/*.mdx') +--- + +{files.map((file: any) => )} diff --git a/packages/integrations/mdx/test/mdx-infinite-loop.test.js b/packages/integrations/mdx/test/mdx-infinite-loop.test.js new file mode 100644 index 0000000000..c76c242703 --- /dev/null +++ b/packages/integrations/mdx/test/mdx-infinite-loop.test.js @@ -0,0 +1,30 @@ +import mdx from '@astrojs/mdx'; + +import { expect } from 'chai'; +import { loadFixture } from '../../../astro/test/test-utils.js'; + +describe('MDX Infinite Loop', () => { + let fixture; + + before(async () => { + fixture = await loadFixture({ + root: new URL('./fixtures/mdx-infinite-loop/', import.meta.url), + integrations: [mdx()], + }); + }); + + describe('build', () => { + let err; + before(async () => { + try { + await fixture.build(); + } catch (e) { + err = e; + } + }); + + it('does not hang forever if an error is thrown', async () => { + expect(!!err).to.be.true; + }); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a25d9fea17..e47fe2fdab 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2729,6 +2729,18 @@ importers: reading-time: 1.5.0 unist-util-visit: 4.1.1 + packages/integrations/mdx/test/fixtures/mdx-infinite-loop: + specifiers: + '@astrojs/mdx': workspace:* + '@astrojs/preact': workspace:* + astro: workspace:* + preact: ^10.7.3 + dependencies: + '@astrojs/mdx': link:../../.. + '@astrojs/preact': link:../../../../preact + astro: link:../../../../../astro + preact: 10.11.0 + packages/integrations/mdx/test/fixtures/mdx-namespace: specifiers: '@astrojs/mdx': workspace:*