Skip to content

Commit

Permalink
Stringify shouldn't throw on user object during rendering (#8127)
Browse files Browse the repository at this point in the history
* fix(#7923): do not throw on user { type } object

* chore: remove unused type export

* chore: guess it wasn't unused
  • Loading branch information
natemoo-re committed Aug 18, 2023
1 parent c7de971 commit b12c847
Show file tree
Hide file tree
Showing 9 changed files with 115 additions and 46 deletions.
5 changes: 5 additions & 0 deletions .changeset/brave-cheetahs-float.md
@@ -0,0 +1,5 @@
---
'astro': patch
---

Do not throw Error when users pass an object with a "type" property
38 changes: 18 additions & 20 deletions 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,
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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<RenderDestinationChunk, Response>) {
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/src/runtime/server/render/component.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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)));
},
};
Expand Down
7 changes: 4 additions & 3 deletions 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
Expand Down Expand Up @@ -45,7 +46,7 @@ export function renderAllHeadContent(result: SSRResult) {
}

export function* renderHead(): Generator<RenderHeadInstruction> {
yield { type: 'head' };
yield createRenderInstruction({ type: 'head' });
}

// This function is called by Astro components that do not contain a <head> component
Expand All @@ -55,5 +56,5 @@ export function* renderHead(): Generator<RenderHeadInstruction> {
export function* maybeRenderHead(): Generator<MaybeRenderHeadInstruction> {
// 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' });
}
2 changes: 1 addition & 1 deletion 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';
Expand All @@ -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';
32 changes: 32 additions & 0 deletions 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];
}
2 changes: 1 addition & 1 deletion 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';
Expand Down
19 changes: 0 additions & 19 deletions packages/astro/src/runtime/server/render/types.ts

This file was deleted.

52 changes: 52 additions & 0 deletions 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' }
---
<div id="chunk">{value}</div>
`,
},
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;
}
}
);
});
});

0 comments on commit b12c847

Please sign in to comment.