Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stringify shouldn't throw on user object during rendering #8127

Merged
merged 5 commits into from
Aug 18, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/brave-cheetahs-float.md
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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;
}
}
);
});
});