diff --git a/.changeset/warm-plums-grin.md b/.changeset/warm-plums-grin.md index 0faa554f11c..6581edc8a8b 100644 --- a/.changeset/warm-plums-grin.md +++ b/.changeset/warm-plums-grin.md @@ -2,6 +2,8 @@ '@whatwg-node/node-fetch': patch --- +- Normalization fixes on `URL` + Normalize the URL if it has a port and query string without a pathname; ```diff @@ -14,4 +16,39 @@ Previously, it was normalized like below which was incorrect; ```diff - http://example.com:80?query + http://example.com/:80?query -``` \ No newline at end of file +``` + +- Fix `URL.origin` + +When the URL has a port, `origin` was doubling the port number; + +```diff +- http://example.com:80:80 ++ http://example.com:80 +``` + +- Fix `ReadableStream[Symbol.asyncIterator]` + +`ReadableStream` uses `Readable` so it uses `Symbol.asyncIterator` method of `Readable` but the returned iterator's `.return` method doesn't handle cancellation correctly. So we need to call `readable.destroy(optionalError)` manually to cancel the stream. + +This allows `ReadableStream` to use implementations relying on `AsyncIterable.cancel` to handle cancellation like `Readable.from` + +Previously the following was not handling cancellation; + +```ts +const res = new ReadableStream({ + start(controller) { + controller.enqueue('Hello'); + controller.enqueue('World'); + }, + cancel(reason) { + console.log('cancelled', reason); + } +}); + +const readable = Readable.from(res); + +readable.destroy(new Error('MY REASON')); + +// Should log 'cancelled MY REASON' +``` diff --git a/packages/node-fetch/src/ReadableStream.ts b/packages/node-fetch/src/ReadableStream.ts index 76fe50f61b1..51afa79b098 100644 --- a/packages/node-fetch/src/ReadableStream.ts +++ b/packages/node-fetch/src/ReadableStream.ts @@ -175,13 +175,13 @@ export class PonyfillReadableStream implements ReadableStream { if (!this.readable.destroyed) { this.readable.destroy(); } - return iterator.return?.(); + return iterator.return?.() || fakePromise({ done: true, value: undefined }); }, throw: (err: Error) => { if (!this.readable.destroyed) { this.readable.destroy(err); } - return iterator.throw?.(err); + return iterator.throw?.(err) || fakePromise({ done: true, value: undefined }); }, }; } diff --git a/packages/server/test/node.spec.ts b/packages/server/test/node.spec.ts index 0d4cadb2405..411fdbc0c41 100644 --- a/packages/server/test/node.spec.ts +++ b/packages/server/test/node.spec.ts @@ -383,10 +383,6 @@ describe('Node Specific Cases', () => { const received: { status: number; statusText: string; resText?: string }[] = []; for (const statusCodeStr in STATUS_CODES) { const status = Number(statusCodeStr); - if (serverImplName === 'koa' && status === 425) { - // Koa gives 'Unordered Collection' for 425 - continue; - } if (status < 200) { // Informational responses are not supported by fetch in this way continue; @@ -420,13 +416,18 @@ describe('Node Specific Cases', () => { const res = await fetch(testServer.url, { signal: ctrl.signal, }); - const expectedStatusText = STATUS_CODES[status]; + let expectedStatusText = STATUS_CODES[status]; + // Koa gives 'Unordered Collection' for 425 + if (status === 425 && serverImplName === 'koa') { + expectedStatusText = 'Unordered Collection'; + } if (!expectedStatusText) { throw new Error(`No status text for status ${status}`); } resText = await res.text(); expected.push({ status, + // Some servers like Koa has a different naming convention for status texts statusText: expectedStatusText.toLowerCase(), }); received.push({ diff --git a/packages/server/test/request-listener.spec.ts b/packages/server/test/request-listener.spec.ts index 6ca26600dec..0baa8a0a99f 100644 --- a/packages/server/test/request-listener.spec.ts +++ b/packages/server/test/request-listener.spec.ts @@ -106,9 +106,7 @@ describe('Request Listener', () => { const { 'content-length': contentLength1, ...toBeCheckedObj } = getHeadersObj( toBeChecked.headers, ); - const { 'content-length': contentLength2, ...expectedObj } = getHeadersObj( - expected.headers, - ); + const { 'content-length': contentLength2, ...expectedObj } = getHeadersObj(expected.headers); expect(toBeCheckedObj).toMatchObject(expectedObj); } @@ -116,9 +114,7 @@ describe('Request Listener', () => { const { 'content-length': contentLength1, ...toBeCheckedObj } = getHeadersObj( toBeChecked.headers, ); - const { 'content-length': contentLength2, ...expectedObj } = getHeadersObj( - expected.headers, - ); + const { 'content-length': contentLength2, ...expectedObj } = getHeadersObj(expected.headers); expect(toBeCheckedObj).toMatchObject(expectedObj); expect(toBeChecked.status).toBe(expected.status); }