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 <nate@astro.build>
This commit is contained in:
parent
0ae8147988
commit
cf2bba1e4a
10 changed files with 84 additions and 1 deletions
5
.changeset/tall-files-smash.md
Normal file
5
.changeset/tall-files-smash.md
Normal file
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
'astro': patch
|
||||||
|
---
|
||||||
|
|
||||||
|
Fix MDX error handling, preventing a memory leak
|
|
@ -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 AstroJSX = 'astro:jsx';
|
||||||
const Empty = Symbol('empty');
|
const Empty = Symbol('empty');
|
||||||
|
|
||||||
export interface AstroVNode {
|
export interface AstroVNode {
|
||||||
|
[Renderer]: string;
|
||||||
[AstroJSX]: boolean;
|
[AstroJSX]: boolean;
|
||||||
type: string | ((...args: any) => any);
|
type: string | ((...args: any) => any);
|
||||||
props: Record<string, any>;
|
props: Record<string, any>;
|
||||||
|
@ -74,6 +75,7 @@ function transformSetDirectives(vnode: AstroVNode) {
|
||||||
|
|
||||||
function createVNode(type: any, props: Record<string, any>) {
|
function createVNode(type: any, props: Record<string, any>) {
|
||||||
const vnode: AstroVNode = {
|
const vnode: AstroVNode = {
|
||||||
|
[Renderer]: 'astro:jsx',
|
||||||
[AstroJSX]: true,
|
[AstroJSX]: true,
|
||||||
type,
|
type,
|
||||||
props: props ?? {},
|
props: props ?? {},
|
||||||
|
|
|
@ -38,6 +38,10 @@ export async function renderJSX(result: SSRResult, vnode: any): Promise<any> {
|
||||||
}
|
}
|
||||||
if (isVNode(vnode)) {
|
if (isVNode(vnode)) {
|
||||||
switch (true) {
|
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'):
|
case (vnode.type as any) === Symbol.for('astro:fragment'):
|
||||||
return renderJSX(result, vnode.props.children);
|
return renderJSX(result, vnode.props.children);
|
||||||
case (vnode.type as any).isAstroComponentFactory: {
|
case (vnode.type as any).isAstroComponentFactory: {
|
||||||
|
|
6
packages/integrations/mdx/test/fixtures/mdx-infinite-loop/astro.config.ts
vendored
Normal file
6
packages/integrations/mdx/test/fixtures/mdx-infinite-loop/astro.config.ts
vendored
Normal file
|
@ -0,0 +1,6 @@
|
||||||
|
import mdx from '@astrojs/mdx';
|
||||||
|
import preact from '@astrojs/preact';
|
||||||
|
|
||||||
|
export default {
|
||||||
|
integrations: [mdx(), preact()]
|
||||||
|
}
|
10
packages/integrations/mdx/test/fixtures/mdx-infinite-loop/package.json
vendored
Normal file
10
packages/integrations/mdx/test/fixtures/mdx-infinite-loop/package.json
vendored
Normal file
|
@ -0,0 +1,10 @@
|
||||||
|
{
|
||||||
|
"name": "@test/mdx-infinite-loop",
|
||||||
|
"type": "module",
|
||||||
|
"dependencies": {
|
||||||
|
"@astrojs/mdx": "workspace:*",
|
||||||
|
"@astrojs/preact": "workspace:*",
|
||||||
|
"preact": "^10.7.3",
|
||||||
|
"astro": "workspace:*"
|
||||||
|
}
|
||||||
|
}
|
3
packages/integrations/mdx/test/fixtures/mdx-infinite-loop/src/components/Test.js
vendored
Normal file
3
packages/integrations/mdx/test/fixtures/mdx-infinite-loop/src/components/Test.js
vendored
Normal file
|
@ -0,0 +1,3 @@
|
||||||
|
export default function () {
|
||||||
|
return 'Hello world'
|
||||||
|
}
|
6
packages/integrations/mdx/test/fixtures/mdx-infinite-loop/src/pages/doc.mdx
vendored
Normal file
6
packages/integrations/mdx/test/fixtures/mdx-infinite-loop/src/pages/doc.mdx
vendored
Normal file
|
@ -0,0 +1,6 @@
|
||||||
|
import Test, { Missing } from '../components/Test';
|
||||||
|
|
||||||
|
# Hello page!
|
||||||
|
|
||||||
|
<Test />
|
||||||
|
<Missing />
|
5
packages/integrations/mdx/test/fixtures/mdx-infinite-loop/src/pages/index.astro
vendored
Normal file
5
packages/integrations/mdx/test/fixtures/mdx-infinite-loop/src/pages/index.astro
vendored
Normal file
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
const files = await Astro.glob('./**/*.mdx')
|
||||||
|
---
|
||||||
|
|
||||||
|
{files.map((file: any) => <file.Content />)}
|
30
packages/integrations/mdx/test/mdx-infinite-loop.test.js
Normal file
30
packages/integrations/mdx/test/mdx-infinite-loop.test.js
Normal file
|
@ -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;
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
12
pnpm-lock.yaml
generated
12
pnpm-lock.yaml
generated
|
@ -2729,6 +2729,18 @@ importers:
|
||||||
reading-time: 1.5.0
|
reading-time: 1.5.0
|
||||||
unist-util-visit: 4.1.1
|
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:
|
packages/integrations/mdx/test/fixtures/mdx-namespace:
|
||||||
specifiers:
|
specifiers:
|
||||||
'@astrojs/mdx': workspace:*
|
'@astrojs/mdx': workspace:*
|
||||||
|
|
Loading…
Add table
Reference in a new issue