From b12c8471f413c0291de4a9c444bfe3079a192034 Mon Sep 17 00:00:00 2001 From: Nate Moore Date: Fri, 18 Aug 2023 07:26:52 -0500 Subject: [PATCH] Stringify shouldn't throw on user object during rendering (#8127) * fix(#7923): do not throw on user { type } object * chore: remove unused type export * chore: guess it wasn't unused --- .changeset/brave-cheetahs-float.md | 5 ++ .../astro/src/runtime/server/render/common.ts | 38 +++++++------- .../src/runtime/server/render/component.ts | 4 +- .../astro/src/runtime/server/render/head.ts | 7 +-- .../astro/src/runtime/server/render/index.ts | 2 +- .../src/runtime/server/render/instruction.ts | 32 ++++++++++++ .../astro/src/runtime/server/render/slot.ts | 2 +- .../astro/src/runtime/server/render/types.ts | 19 ------- .../astro/test/units/render/chunk.test.js | 52 +++++++++++++++++++ 9 files changed, 115 insertions(+), 46 deletions(-) create mode 100644 .changeset/brave-cheetahs-float.md create mode 100644 packages/astro/src/runtime/server/render/instruction.ts delete mode 100644 packages/astro/src/runtime/server/render/types.ts create mode 100644 packages/astro/test/units/render/chunk.test.js diff --git a/.changeset/brave-cheetahs-float.md b/.changeset/brave-cheetahs-float.md new file mode 100644 index 000000000000..84825ce0f7d3 --- /dev/null +++ b/.changeset/brave-cheetahs-float.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Do not throw Error when users pass an object with a "type" property diff --git a/packages/astro/src/runtime/server/render/common.ts b/packages/astro/src/runtime/server/render/common.ts index 48d8143df8e6..a49bb2c9dd3c 100644 --- a/packages/astro/src/runtime/server/render/common.ts +++ b/packages/astro/src/runtime/server/render/common.ts @@ -1,6 +1,7 @@ import type { SSRResult } from '../../../@types/astro'; -import type { RenderInstruction } from './types.js'; +import type { RenderInstruction } from './instruction.js'; +import { isRenderInstruction } from './instruction.js'; import { HTMLBytes, HTMLString, markHTMLString } from '../escape.js'; import { determineIfNeedsHydrationScript, @@ -52,8 +53,8 @@ function stringifyChunk( result: SSRResult, chunk: string | HTMLString | SlotString | RenderInstruction ): string { - if (typeof (chunk as any).type === 'string') { - const instruction = chunk as RenderInstruction; + if (isRenderInstruction(chunk)) { + const instruction = chunk; switch (instruction.type) { case 'directive': { const { hydration } = instruction; @@ -64,8 +65,8 @@ function stringifyChunk( let prescriptType: PrescriptType = needsHydrationScript ? 'both' : needsDirectiveScript - ? 'directive' - : null; + ? 'directive' + : null; if (prescriptType) { let prescripts = getPrescripts(result, prescriptType, hydration.directive); return markHTMLString(prescripts); @@ -86,27 +87,24 @@ function stringifyChunk( return renderAllHeadContent(result); } default: { - if (chunk instanceof Response) { - return ''; - } throw new Error(`Unknown chunk type: ${(chunk as any).type}`); } } - } else { - if (isSlotString(chunk as string)) { - let out = ''; - const c = chunk as SlotString; - if (c.instructions) { - for (const instr of c.instructions) { - out += stringifyChunk(result, instr); - } + } else if (chunk instanceof Response) { + return ''; + } else if (isSlotString(chunk as string)) { + let out = ''; + const c = chunk as SlotString; + if (c.instructions) { + for (const instr of c.instructions) { + out += stringifyChunk(result, instr); } - out += chunk.toString(); - return out; } - - return chunk.toString(); + out += chunk.toString(); + return out; } + + return chunk.toString(); } export function chunkToString(result: SSRResult, chunk: Exclude) { diff --git a/packages/astro/src/runtime/server/render/component.ts b/packages/astro/src/runtime/server/render/component.ts index f78f11d517f7..5f61e68b5034 100644 --- a/packages/astro/src/runtime/server/render/component.ts +++ b/packages/astro/src/runtime/server/render/component.ts @@ -4,7 +4,7 @@ import type { SSRLoadedRenderer, SSRResult, } from '../../../@types/astro'; -import type { RenderInstruction } from './types.js'; +import { createRenderInstruction, type RenderInstruction } from './instruction.js'; import { AstroError, AstroErrorData } from '../../../core/errors/index.js'; import { HTMLBytes, markHTMLString } from '../escape.js'; @@ -370,7 +370,7 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr destination.write(instruction); } } - destination.write({ type: 'directive', hydration }); + destination.write(createRenderInstruction({ type: 'directive', hydration })); destination.write(markHTMLString(renderElement('astro-island', island, false))); }, }; diff --git a/packages/astro/src/runtime/server/render/head.ts b/packages/astro/src/runtime/server/render/head.ts index 7bb38f07db5f..d56684ce58f9 100644 --- a/packages/astro/src/runtime/server/render/head.ts +++ b/packages/astro/src/runtime/server/render/head.ts @@ -1,7 +1,8 @@ import type { SSRResult } from '../../../@types/astro'; import { markHTMLString } from '../escape.js'; -import type { MaybeRenderHeadInstruction, RenderHeadInstruction } from './types'; +import { createRenderInstruction } from './instruction.js'; +import type { MaybeRenderHeadInstruction, RenderHeadInstruction } from './instruction.js'; import { renderElement } from './util.js'; // Filter out duplicate elements in our set @@ -45,7 +46,7 @@ export function renderAllHeadContent(result: SSRResult) { } export function* renderHead(): Generator { - yield { type: 'head' }; + yield createRenderInstruction({ type: 'head' }); } // This function is called by Astro components that do not contain a component @@ -55,5 +56,5 @@ export function* renderHead(): Generator { export function* maybeRenderHead(): Generator { // This is an instruction informing the page rendering that head might need rendering. // This allows the page to deduplicate head injections. - yield { type: 'maybe-head' }; + yield createRenderInstruction({ type: 'maybe-head' }); } diff --git a/packages/astro/src/runtime/server/render/index.ts b/packages/astro/src/runtime/server/render/index.ts index 70d63ca600eb..c7e39e70a005 100644 --- a/packages/astro/src/runtime/server/render/index.ts +++ b/packages/astro/src/runtime/server/render/index.ts @@ -1,4 +1,5 @@ export type { AstroComponentFactory, AstroComponentInstance } from './astro/index'; +export type { RenderInstruction } from './instruction'; export { createHeadAndContent, renderTemplate, renderToString } from './astro/index.js'; export { Fragment, Renderer, chunkToByteArray, chunkToString } from './common.js'; export { renderComponent, renderComponentToString } from './component.js'; @@ -7,5 +8,4 @@ export { maybeRenderHead, renderHead } from './head.js'; export { renderPage } from './page.js'; export { renderSlot, renderSlotToString, type ComponentSlots } from './slot.js'; export { renderScriptElement, renderUniqueStylesheet } from './tags.js'; -export type { RenderInstruction } from './types'; export { addAttribute, defineScriptVars, voidElementNames } from './util.js'; diff --git a/packages/astro/src/runtime/server/render/instruction.ts b/packages/astro/src/runtime/server/render/instruction.ts new file mode 100644 index 000000000000..5cda5cec99ce --- /dev/null +++ b/packages/astro/src/runtime/server/render/instruction.ts @@ -0,0 +1,32 @@ +import type { HydrationMetadata } from '../hydration.js'; + +const RenderInstructionSymbol = Symbol.for('astro:render'); + +export type RenderDirectiveInstruction = { + type: 'directive'; + hydration: HydrationMetadata; +}; + +export type RenderHeadInstruction = { + type: 'head'; +}; + +export type MaybeRenderHeadInstruction = { + type: 'maybe-head'; +}; + +export type RenderInstruction = + | RenderDirectiveInstruction + | RenderHeadInstruction + | MaybeRenderHeadInstruction; + +export function createRenderInstruction(instruction: RenderDirectiveInstruction): RenderDirectiveInstruction; +export function createRenderInstruction(instruction: RenderHeadInstruction): RenderHeadInstruction; +export function createRenderInstruction(instruction: MaybeRenderHeadInstruction): MaybeRenderHeadInstruction; +export function createRenderInstruction(instruction: { type: string }): RenderInstruction { + return Object.defineProperty(instruction as RenderInstruction, RenderInstructionSymbol, { value: true }); +} + +export function isRenderInstruction(chunk: any): chunk is RenderInstruction { + return chunk && typeof chunk === 'object' && chunk[RenderInstructionSymbol]; +} diff --git a/packages/astro/src/runtime/server/render/slot.ts b/packages/astro/src/runtime/server/render/slot.ts index daae87a80784..e887b19d2b41 100644 --- a/packages/astro/src/runtime/server/render/slot.ts +++ b/packages/astro/src/runtime/server/render/slot.ts @@ -1,6 +1,6 @@ import type { SSRResult } from '../../../@types/astro.js'; import type { renderTemplate } from './astro/render-template.js'; -import type { RenderInstruction } from './types.js'; +import type { RenderInstruction } from './instruction.js'; import { HTMLString, markHTMLString } from '../escape.js'; import { renderChild } from './any.js'; diff --git a/packages/astro/src/runtime/server/render/types.ts b/packages/astro/src/runtime/server/render/types.ts deleted file mode 100644 index 1c3a888db1e8..000000000000 --- a/packages/astro/src/runtime/server/render/types.ts +++ /dev/null @@ -1,19 +0,0 @@ -import type { HydrationMetadata } from '../hydration.js'; - -export type RenderDirectiveInstruction = { - type: 'directive'; - hydration: HydrationMetadata; -}; - -export type RenderHeadInstruction = { - type: 'head'; -}; - -export type MaybeRenderHeadInstruction = { - type: 'maybe-head'; -}; - -export type RenderInstruction = - | RenderDirectiveInstruction - | RenderHeadInstruction - | MaybeRenderHeadInstruction; diff --git a/packages/astro/test/units/render/chunk.test.js b/packages/astro/test/units/render/chunk.test.js new file mode 100644 index 000000000000..b623f662f449 --- /dev/null +++ b/packages/astro/test/units/render/chunk.test.js @@ -0,0 +1,52 @@ +import { expect } from 'chai'; +import * as cheerio from 'cheerio'; +import { fileURLToPath } from 'node:url'; +import { createFs, createRequestAndResponse, runInContainer } from '../test-utils.js'; + +const root = new URL('../../fixtures/alias/', import.meta.url); + +describe('core/render chunk', () => { + it('does not throw on user object with type', async () => { + const fs = createFs( + { + '/src/pages/index.astro': ` + --- + const value = { type: 'foobar' } + --- +
{value}
+ `, + }, + root + ); + + await runInContainer( + { + fs, + inlineConfig: { + root: fileURLToPath(root), + logLevel: 'silent', + integrations: [], + }, + }, + async (container) => { + const { req, res, done, text } = createRequestAndResponse({ + method: 'GET', + url: '/', + }); + container.handle(req, res); + + await done; + try { + const html = await text(); + const $ = cheerio.load(html); + const target = $('#chunk'); + + expect(target).not.to.be.undefined; + expect(target.text()).to.equal('[object Object]'); + } catch (e) { + expect(false).to.be.ok; + } + } + ); + }); +});