diff --git a/.changeset/hot-parrots-exist.md b/.changeset/hot-parrots-exist.md new file mode 100644 index 000000000..f7c663987 --- /dev/null +++ b/.changeset/hot-parrots-exist.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Prevent jsx throws from hanging server diff --git a/packages/astro/src/jsx-runtime/index.ts b/packages/astro/src/jsx-runtime/index.ts index bbef1f2f5..86d96494a 100644 --- a/packages/astro/src/jsx-runtime/index.ts +++ b/packages/astro/src/jsx-runtime/index.ts @@ -7,7 +7,7 @@ export interface AstroVNode { [Renderer]: string; [AstroJSX]: boolean; type: string | ((...args: any) => any); - props: Record; + props: Record; } const toSlotName = (slotAttr: string) => slotAttr; diff --git a/packages/astro/src/runtime/server/hydration.ts b/packages/astro/src/runtime/server/hydration.ts index 24552cf30..36e7ca287 100644 --- a/packages/astro/src/runtime/server/hydration.ts +++ b/packages/astro/src/runtime/server/hydration.ts @@ -23,14 +23,14 @@ export interface HydrationMetadata { interface ExtractedProps { isPage: boolean; hydration: HydrationMetadata | null; - props: Record; + props: Record; } // Used to extract the directives, aka `client:load` information about a component. // Finds these special props and removes them from what gets passed into the component. export function extractDirectives( displayName: string, - inputProps: Record + inputProps: Record ): ExtractedProps { let extracted: ExtractedProps = { isPage: false, @@ -105,6 +105,9 @@ export function extractDirectives( extracted.props[key] = value; } } + for(const sym of Object.getOwnPropertySymbols(inputProps)) { + extracted.props[sym] = inputProps[sym]; + } return extracted; } diff --git a/packages/astro/src/runtime/server/jsx.ts b/packages/astro/src/runtime/server/jsx.ts index 05efe2b31..96a15299c 100644 --- a/packages/astro/src/runtime/server/jsx.ts +++ b/packages/astro/src/runtime/server/jsx.ts @@ -1,6 +1,6 @@ /* eslint-disable no-console */ import { SSRResult } from '../../@types/astro.js'; -import { AstroJSX, isVNode } from '../../jsx-runtime/index.js'; +import { AstroJSX, AstroVNode, isVNode } from '../../jsx-runtime/index.js'; import { escapeHTML, HTMLString, @@ -15,7 +15,26 @@ import type { ComponentIterable } from './render/component'; const ClientOnlyPlaceholder = 'astro-client-only'; -const skipAstroJSXCheck = new WeakSet(); +class Skip { + count: number; + constructor(public vnode: AstroVNode) { + this.count = 0; + } + + increment() { + this.count++; + } + + haveNoTried() { + return this.count === 0; + } + + isCompleted() { + return this.count > 2; + } + static symbol = Symbol('astro:jsx:skip'); +} + let originalConsoleError: any; let consoleFilterRefs = 0; @@ -37,6 +56,19 @@ export async function renderJSX(result: SSRResult, vnode: any): Promise { (await Promise.all(vnode.map((v: any) => renderJSX(result, v)))).join('') ); } + + // Extract the skip from the props, if we've already attempted a previous render + let skip: Skip; + if(vnode.props[Skip.symbol]) { + skip = vnode.props[Skip.symbol]; + } else { + skip = new Skip(vnode); + } + + return renderJSXVNode(result, vnode, skip); +} + +async function renderJSXVNode(result: SSRResult, vnode: AstroVNode, skip: Skip): Promise { if (isVNode(vnode)) { switch (true) { case !vnode.type: { @@ -68,25 +100,36 @@ Did you forget to import the component or is it possible there is a typo?`); if (vnode.type) { if (typeof vnode.type === 'function' && (vnode.type as any)['astro:renderer']) { - skipAstroJSXCheck.add(vnode.type); + skip.increment(); } if (typeof vnode.type === 'function' && vnode.props['server:root']) { const output = await vnode.type(vnode.props ?? {}); return await renderJSX(result, output); } - if (typeof vnode.type === 'function' && !skipAstroJSXCheck.has(vnode.type)) { - useConsoleFilter(); - try { - const output = await vnode.type(vnode.props ?? {}); - if (output && output[AstroJSX]) { - return await renderJSX(result, output); - } else if (!output) { - return await renderJSX(result, output); + if (typeof vnode.type === 'function') { + if(skip.haveNoTried() || skip.isCompleted()) { + useConsoleFilter(); + try { + const output = await vnode.type(vnode.props ?? {}); + let renderResult: any; + if (output && output[AstroJSX]) { + renderResult = await renderJSXVNode(result, output, skip); + return renderResult; + } else if (!output) { + renderResult = await renderJSXVNode(result, output, skip); + return renderResult; + } + + } catch (e: unknown) { + if(skip.isCompleted()) { + throw e; + } + skip.increment(); + } finally { + finishUsingConsoleFilter(); } - } catch (e) { - skipAstroJSXCheck.add(vnode.type); - } finally { - finishUsingConsoleFilter(); + } else { + skip.increment(); } } @@ -128,6 +171,7 @@ Did you forget to import the component or is it possible there is a typo?`); } await Promise.all(slotPromises); + props[Skip.symbol] = skip; let output: ComponentIterable; if (vnode.type === ClientOnlyPlaceholder && vnode.props['client:only']) { output = await renderComponent( diff --git a/packages/astro/test/units/render/jsx.test.js b/packages/astro/test/units/render/jsx.test.js index 5cafd582c..dd907725d 100644 --- a/packages/astro/test/units/render/jsx.test.js +++ b/packages/astro/test/units/render/jsx.test.js @@ -1,6 +1,6 @@ import { expect } from 'chai'; -import { createComponent, render, renderSlot } from '../../../dist/runtime/server/index.js'; +import { createComponent, render, renderComponent, renderSlot } from '../../../dist/runtime/server/index.js'; import { jsx } from '../../../dist/jsx-runtime/index.js'; import { createBasicEnvironment, @@ -90,5 +90,25 @@ describe('core/render', () => { '

works

works

' ); }); + + it('Errors in JSX components are raised', async () => { + const Component = createAstroJSXComponent(() => { + throw new Error('uh oh'); + }); + + const Page = createComponent((result, _props) => { + return render`
${renderComponent(result, 'Component', Component, {})}
`; + }); + + const ctx = createRenderContext({ request: new Request('http://example.com/') }); + const response = await renderPage(createAstroModule(Page), ctx, env); + + try { + await response.text(); + expect(false).to.equal(true, 'should not have been successful'); + } catch(err) { + expect(err.message).to.equal('uh oh'); + } + }); }); });