Skip to content

Commit

Permalink
Adding an option to disable HTTP streaming (withastro#3777)
Browse files Browse the repository at this point in the history
* Adding a flag to disable HTTP streaming

* refactor: adding support for SSG builds

* handling string responses in the server runtime, adding tests

* removing streaming CLI flag

* removing import.meta.env.STREAMING

* include Content-Length header when streaming is disabled

* Verifying content-length header in dev

* fix: default streaming to enabled in the base App server

* TEMP: disabling the production test to investigate the test-adapter

* re-enabling the test with an adapter option to disable streaming for the test

* fix: use the existing TextEncoder to get the body's byte length

* moving config to build.streaming, ignoring it in `dev`

* fixing dev test to expect response streaming

* chore: add changsets

* removing the new config option all together 🎉

* remove temp debug log

* Updating astro changeset now that streaming isn't a config option
  • Loading branch information
Tony Sullivan authored Jul 1, 2022
1 parent d8a703f commit 8efdc09
Show file tree
Hide file tree
Showing 12 changed files with 135 additions and 25 deletions.
5 changes: 4 additions & 1 deletion src/core/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,16 @@ export class App {
dest: consoleLogDestination,
level: 'info',
};
#streaming: boolean;

constructor(manifest: Manifest) {
constructor(manifest: Manifest, streaming = true) {
this.#manifest = manifest;
this.#manifestData = {
routes: manifest.routes.map((route) => route.routeData),
};
this.#routeDataToRouteInfo = new Map(manifest.routes.map((route) => [route.routeData, route]));
this.#routeCache = new RouteCache(this.#logging);
this.#streaming = streaming;
}
match(request: Request): RouteData | undefined {
const url = new URL(request.url);
Expand Down Expand Up @@ -117,6 +119,7 @@ export class App {
site: this.#manifest.site,
ssr: true,
request,
streaming: this.#streaming,
});

return response;
Expand Down
1 change: 1 addition & 0 deletions src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ async function generatePath(
? new URL(astroConfig.base, astroConfig.site).toString()
: astroConfig.site,
ssr,
streaming: true,
};

let body: string;
Expand Down
2 changes: 2 additions & 0 deletions src/core/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const ASTRO_CONFIG_DEFAULTS: AstroUserConfig & any = {
server: {
host: false,
port: 3000,
streaming: true,
},
style: { postcss: { options: {}, plugins: [] } },
integrations: [],
Expand Down Expand Up @@ -315,6 +316,7 @@ export async function validateConfig(
.optional()
.default(ASTRO_CONFIG_DEFAULTS.server.host),
port: z.number().optional().default(ASTRO_CONFIG_DEFAULTS.server.port),
streaming: z.boolean().optional().default(true),
})
.optional()
.default({})
Expand Down
5 changes: 4 additions & 1 deletion src/core/render/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export interface RenderOptions {
routeCache: RouteCache;
site?: string;
ssr: boolean;
streaming: boolean;
request: Request;
}

Expand All @@ -100,6 +101,7 @@ export async function render(opts: RenderOptions): Promise<Response> {
routeCache,
site,
ssr,
streaming,
} = opts;

const paramsAndPropsRes = await getParamsAndProps({
Expand Down Expand Up @@ -138,12 +140,13 @@ export async function render(opts: RenderOptions): Promise<Response> {
site,
scripts,
ssr,
streaming,
});

// Support `export const components` for `MDX` pages
if (typeof (mod as any).components === 'object') {
Object.assign(pageProps, { components: (mod as any).components });
}

return await renderPage(result, Component, pageProps, null);
return await renderPage(result, Component, pageProps, null, streaming);
}
1 change: 1 addition & 0 deletions src/core/render/dev/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ export async function render(
routeCache,
site: astroConfig.site ? new URL(astroConfig.base, astroConfig.site).toString() : undefined,
ssr: isBuildingToSSR(astroConfig),
streaming: true,
});

return response;
Expand Down
7 changes: 6 additions & 1 deletion src/core/render/result.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ function onlyAvailableInSSR(name: string) {

export interface CreateResultArgs {
ssr: boolean;
streaming: boolean;
logging: LogOptions;
origin: string;
markdown: MarkdownRenderingOptions;
Expand Down Expand Up @@ -114,7 +115,11 @@ export function createResult(args: CreateResultArgs): SSRResult {
const url = new URL(request.url);
const canonicalURL = createCanonicalURL('.' + pathname, site ?? url.origin, paginated);
const headers = new Headers();
headers.set('Transfer-Encoding', 'chunked');
if (args.streaming) {
headers.set('Transfer-Encoding', 'chunked');
} else {
headers.set('Content-Type', 'text/html');
}
const response: ResponseInit = {
status: 200,
statusText: 'OK',
Expand Down
59 changes: 40 additions & 19 deletions src/runtime/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,8 @@ export async function renderPage(
result: SSRResult,
componentFactory: AstroComponentFactory,
props: any,
children: any
children: any,
streaming: boolean,
): Promise<Response> {
let iterable: AsyncIterable<any>;
if (!componentFactory.isAstroComponentFactory) {
Expand All @@ -730,28 +731,48 @@ export async function renderPage(
const factoryReturnValue = await componentFactory(result, props, children);

if (isAstroComponent(factoryReturnValue)) {
iterable = renderAstroComponent(factoryReturnValue);
let stream = new ReadableStream({
start(controller) {
async function read() {
let i = 0;
for await (const chunk of iterable) {
let html = chunk.toString();
if (i === 0) {
if (!/<!doctype html/i.test(html)) {
controller.enqueue(encoder.encode('<!DOCTYPE html>\n'));
let iterable = renderAstroComponent(factoryReturnValue);
let init = result.response;
let headers = new Headers(init.headers);
let body: BodyInit;
if (streaming) {
body = new ReadableStream({
start(controller) {
async function read() {
let i = 0;
for await (const chunk of iterable) {
let html = chunk.toString();
if (i === 0) {
if (!/<!doctype html/i.test(html)) {
controller.enqueue(encoder.encode('<!DOCTYPE html>\n'));
}
}
controller.enqueue(encoder.encode(html));
i++;
}
controller.enqueue(encoder.encode(html));
i++;
controller.close();
}
read();
},
});
} else {
body = '';
let i = 0;
for await (const chunk of iterable) {
let html = chunk.toString();
if (i === 0) {
if (!/<!doctype html/i.test(html)) {
body += '<!DOCTYPE html>\n';
}
controller.close();
}
read();
},
});
let init = result.response;
let response = createResponse(stream, init);
body += chunk;
i++;
}
const bytes = encoder.encode(body);
headers.set('Content-Length', `${bytes.byteLength}`);
}

let response = createResponse(body, { ...init, headers });
return response;
} else {
return factoryReturnValue;
Expand Down
3 changes: 3 additions & 0 deletions src/runtime/server/response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ type CreateResponseFn = (body?: BodyInit | null, init?: ResponseInit) => Respons

export const createResponse: CreateResponseFn = isNodeJS
? (body, init) => {
if (typeof body === 'string') {
return new Response(body, init);
}
if (typeof StreamingCompatibleResponse === 'undefined') {
return new (createResponseClass())(body, init);
}
Expand Down
2 changes: 2 additions & 0 deletions src/vite-plugin-astro-server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ async function writeWebResponse(res: http.ServerResponse, webResponse: Response)
} else if (body instanceof Readable) {
body.pipe(res);
return;
} else if (typeof body === 'string') {
res.write(body);
} else {
const reader = body.getReader();
while (true) {
Expand Down
69 changes: 69 additions & 0 deletions test/streaming.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,72 @@ describe('Streaming', () => {
});
});
});

describe('Streaming disabled', () => {
if (isWindows) return;

/** @type {import('./test-utils').Fixture} */
let fixture;

before(async () => {
fixture = await loadFixture({
root: './fixtures/streaming/',
adapter: testAdapter(),
experimental: {
ssr: true,
},
server: {
streaming: false,
}
});
});

describe('Development', () => {
/** @type {import('./test-utils').DevServer} */
let devServer;

before(async () => {
devServer = await fixture.startDevServer();
});

after(async () => {
await devServer.stop();
});

it('Body is chunked', async () => {
let res = await fixture.fetch('/');
let chunks = [];
for await (const bytes of res.body) {
let chunk = bytes.toString('utf-8');
chunks.push(chunk);
}
expect(chunks.length).to.be.greaterThan(1);
});
});

// TODO: find a different solution for the test-adapter,
// currently there's no way to resolve two different versions with one
// having streaming disabled
describe('Production', () => {
before(async () => {
await fixture.build();
});

it('Can get the full html body', async () => {
const app = await fixture.loadTestAdapterApp(false);
const request = new Request('http://example.com/');
const response = await app.render(request);

expect(response.status).to.equal(200);
expect(response.headers.get('content-type')).to.equal('text/html');
expect(response.headers.has('content-length')).to.equal(true);
expect(parseInt(response.headers.get('content-length'))).to.be.greaterThan(0);

const html = await response.text();
const $ = cheerio.load(html);

expect($('header h1')).to.have.a.lengthOf(1);
expect($('ul li')).to.have.a.lengthOf(10);
});
});
});
2 changes: 1 addition & 1 deletion test/test-adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export default function () {
},
load(id) {
if (id === '@my-ssr') {
return `import { App } from 'astro/app';export function createExports(manifest) { return { manifest, createApp: () => new App(manifest) }; }`;
return `import { App } from 'astro/app';export function createExports(manifest) { return { manifest, createApp: (streaming) => new App(manifest, streaming) }; }`;
}
},
},
Expand Down
4 changes: 2 additions & 2 deletions test/test-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,10 @@ export async function loadFixture(inlineConfig) {
clean: async () => {
await fs.promises.rm(config.outDir, { maxRetries: 10, recursive: true, force: true });
},
loadTestAdapterApp: async () => {
loadTestAdapterApp: async (streaming) => {
const url = new URL('./server/entry.mjs', config.outDir);
const { createApp, manifest } = await import(url);
const app = createApp();
const app = createApp(streaming);
app.manifest = manifest;
return app;
},
Expand Down

0 comments on commit 8efdc09

Please sign in to comment.