Skip to content

Commit

Permalink
Revert "Undici (#5117)"
Browse files Browse the repository at this point in the history
This reverts commit 7ed3c95.
  • Loading branch information
Rich-Harris committed Jun 28, 2022
1 parent 7ed3c95 commit a8d585a
Show file tree
Hide file tree
Showing 15 changed files with 71 additions and 117 deletions.
5 changes: 0 additions & 5 deletions .changeset/slimy-ways-stare.md

This file was deleted.

4 changes: 0 additions & 4 deletions documentation/docs/01-web-standards.md
Expand Up @@ -45,10 +45,6 @@ export function get(event) {
}
```

### Stream APIs

Most of the time, your endpoints will return complete data, as in the `userAgent` example above. Sometimes, you may need to return a response that's too large to fit in memory in one go, or is delivered in chunks, and for this the platform provides [streams](https://developer.mozilla.org/en-US/docs/Web/API/Streams_API)[ReadableStream](https://developer.mozilla.org/en-US/docs/Web/API/ReadableStream), [WritableStream](https://developer.mozilla.org/en-US/docs/Web/API/WritableStream) and [TransformStream](https://developer.mozilla.org/en-US/docs/Web/API/TransformStream).

### URL APIs

URLs are represented by the [`URL`](https://developer.mozilla.org/en-US/docs/Web/API/URL) interface, which includes useful properties like `origin` and `pathname` (and, in the browser, `hash`). This interface shows up in various places — `event.url` in [hooks](/docs/hooks) and [endpoints](/docs/routing#endpoints), [`$page.url`](/docs/modules#$app-stores) in [pages](/docs/routing#pages), `from` and `to` in [`beforeNavigate` and `afterNavigate`](/docs/modules#$app-navigation) and so on.
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/package.json
Expand Up @@ -33,6 +33,7 @@
"locate-character": "^2.0.5",
"marked": "^4.0.16",
"mime": "^3.0.0",
"node-fetch": "^3.2.4",
"port-authority": "^1.2.0",
"rollup": "^2.75.3",
"selfsigned": "^2.0.1",
Expand All @@ -44,7 +45,6 @@
"svelte2tsx": "~0.5.10",
"tiny-glob": "^0.2.9",
"typescript": "^4.7.2",
"undici": "^5.4.0",
"uvu": "^0.5.3"
},
"peerDependencies": {
Expand Down
39 changes: 7 additions & 32 deletions packages/kit/src/node/index.js
@@ -1,3 +1,4 @@
import { Readable } from 'stream';
import * as set_cookie_parser from 'set-cookie-parser';

/** @param {import('http').IncomingMessage} req */
Expand Down Expand Up @@ -63,7 +64,6 @@ export async function getRequest(base, req) {
delete headers[':authority'];
delete headers[':scheme'];
}

return new Request(base + req.url, {
method: req.method,
headers,
Expand All @@ -85,38 +85,13 @@ export async function setResponse(res, response) {

res.writeHead(response.status, headers);

if (response.body) {
let cancelled = false;

const reader = response.body.getReader();

res.on('close', () => {
reader.cancel();
cancelled = true;
});

const next = async () => {
const { done, value } = await reader.read();

if (cancelled) return;

if (done) {
res.end();
return;
}

res.write(Buffer.from(value), (error) => {
if (error) {
console.error('Error writing stream', error);
res.end();
} else {
next();
}
});
};

next();
if (response.body instanceof Readable) {
response.body.pipe(res);
} else {
if (response.body) {
res.write(new Uint8Array(await response.arrayBuffer()));
}

res.end();
}
}
11 changes: 3 additions & 8 deletions packages/kit/src/node/polyfills.js
@@ -1,5 +1,4 @@
import { fetch, Response, Request, Headers } from 'undici';
import { ReadableStream, TransformStream, WritableStream } from 'stream/web';
import fetch, { Response, Request, Headers } from 'node-fetch';
import { webcrypto as crypto } from 'crypto';

/** @type {Record<string, any>} */
Expand All @@ -8,17 +7,13 @@ const globals = {
fetch,
Response,
Request,
Headers,
ReadableStream,
TransformStream,
WritableStream
Headers
};

// exported for dev/preview and node environments
export function installPolyfills() {
for (const name in globals) {
if (name in globalThis) continue;

// TODO use built-in fetch once https://github.com/nodejs/undici/issues/1262 is resolved
Object.defineProperty(globalThis, name, {
enumerable: true,
configurable: true,
Expand Down
13 changes: 4 additions & 9 deletions packages/kit/src/runtime/server/endpoint.js
Expand Up @@ -21,8 +21,6 @@ const text_types = new Set([
'multipart/form-data'
]);

const bodyless_status_codes = new Set([101, 204, 205, 304]);

/**
* Decides how the body should be parsed based on its mime type
*
Expand Down Expand Up @@ -126,11 +124,8 @@ export async function render_endpoint(event, mod) {
}
}

return new Response(
method !== 'head' && !bodyless_status_codes.has(status) ? normalized_body : undefined,
{
status,
headers
}
);
return new Response(method !== 'head' ? normalized_body : undefined, {
status,
headers
});
}
5 changes: 0 additions & 5 deletions packages/kit/src/runtime/server/page/load_node.js
Expand Up @@ -250,11 +250,6 @@ export async function load_node({
if (cookie) opts.headers.set('cookie', cookie);
}

// we need to delete the connection header, as explained here:
// https://github.com/nodejs/undici/issues/1470#issuecomment-1140798467
// TODO this may be a case for being selective about which headers we let through
opts.headers.delete('connection');

const external_request = new Request(requested, /** @type {RequestInit} */ (opts));
response = await options.hooks.externalFetch.call(null, external_request);
}
Expand Down
10 changes: 3 additions & 7 deletions packages/kit/src/runtime/server/utils.js
Expand Up @@ -39,13 +39,9 @@ export function is_pojo(body) {
if (body) {
if (body instanceof Uint8Array) return false;

// if body is a node Readable, throw an error
// TODO remove this for 1.0
if (body._readableState && typeof body.pipe === 'function') {
throw new Error('Node streams are no longer supported — use a ReadableStream instead');
}

if (body instanceof ReadableStream) return false;
// body could be a node Readable, but we don't want to import
// node built-ins, so we use duck typing
if (body._readableState && typeof body.pipe === 'function') return false;

// similarly, it could be a web ReadableStream
if (typeof ReadableStream !== 'undefined' && body instanceof ReadableStream) return false;
Expand Down
4 changes: 1 addition & 3 deletions packages/kit/src/utils/http.spec.js
@@ -1,9 +1,7 @@
import { test } from 'uvu';
import * as assert from 'uvu/assert';
import { to_headers } from './http.js';
import { Headers } from 'undici';

// @ts-ignore
import { Headers } from 'node-fetch';
globalThis.Headers = Headers;

test('handle header string value', () => {
Expand Down
@@ -1,7 +1,7 @@
<script context="module">
/** @type {import('@sveltejs/kit').Load} */
export async function load({ fetch }) {
const res = await fetch('/load/large-response/text.txt');
export async function load({ url, fetch }) {
const res = await fetch(`http://localhost:${url.searchParams.get('port')}/large-response.json`);
const text = await res.text();
return {
Expand Down

This file was deleted.

41 changes: 39 additions & 2 deletions packages/kit/test/apps/basics/test/test.js
Expand Up @@ -1360,8 +1360,45 @@ test.describe.parallel('Load', () => {
test('handles large responses', async ({ page }) => {
await page.goto('/load');

await page.goto('/load/large-response');
expect(await page.textContent('h1')).toBe('text.length is 5000000');
const chunk_size = 50000;
const chunk_count = 100;
const total_size = chunk_size * chunk_count;

let chunk = '';
for (let i = 0; i < chunk_size; i += 1) {
chunk += String(i % 10);
}

let times_responded = 0;

const { port, server } = await start_server(async (req, res) => {
if (req.url === '/large-response.json') {
times_responded += 1;

res.writeHead(200, {
'Access-Control-Allow-Origin': '*'
});

for (let i = 0; i < chunk_count; i += 1) {
if (!res.write(chunk)) {
await new Promise((fulfil) => {
res.once('drain', () => {
fulfil(undefined);
});
});
}
}

res.end();
}
});

await page.goto(`/load/large-response?port=${port}`);
expect(await page.textContent('h1')).toBe(`text.length is ${total_size}`);

expect(times_responded).toBe(1);

server.close();
});

test('handles external api', async ({ page }) => {
Expand Down
16 changes: 8 additions & 8 deletions packages/kit/test/typings/endpoint.test.ts
Expand Up @@ -51,6 +51,14 @@ export const readable_stream_body: RequestHandler = () => {
};
};

// valid - body instance of stream.Readable should be allowed
export const stream_readable_body: RequestHandler = async () => {
const { Readable } = await import('stream');
return {
body: new Readable()
};
};

// valid - different header pairs should be allowed
export const differential_headers_assignment: RequestHandler = () => {
if (Math.random() < 0.5) {
Expand Down Expand Up @@ -185,11 +193,3 @@ export const error_nested_instances: RequestHandler = () => {
body: { typed: new Uint8Array() }
};
};

// @ts-expect-error - instances cannot be nested
export const stream_readable_body: RequestHandler = async () => {
const { Readable } = await import('stream');
return {
body: new Readable()
};
};
2 changes: 1 addition & 1 deletion packages/kit/types/index.d.ts
Expand Up @@ -268,7 +268,7 @@ export interface ResolveOptions {
transformPage?: ({ html }: { html: string }) => MaybePromise<string>;
}

export type ResponseBody = JSONValue | Uint8Array | ReadableStream;
export type ResponseBody = JSONValue | Uint8Array | ReadableStream | import('stream').Readable;

export class Server {
constructor(manifest: SSRManifest);
Expand Down
9 changes: 2 additions & 7 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit a8d585a

Please sign in to comment.