Skip to content

Commit

Permalink
Run tests with all Node frameworks
Browse files Browse the repository at this point in the history
  • Loading branch information
ardatan committed Dec 27, 2024
1 parent 3419cba commit b82477f
Show file tree
Hide file tree
Showing 13 changed files with 1,015 additions and 418 deletions.
17 changes: 17 additions & 0 deletions .changeset/warm-plums-grin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
'@whatwg-node/node-fetch': patch
---

Normalize the URL if it has a port and query string without a pathname;

```diff
+ http://example.com:80?query
- http://example.com:80/?query
```

Previously, it was normalized like below which was incorrect;

```diff
- http://example.com:80?query
+ http://example.com/:80?query
```
7 changes: 6 additions & 1 deletion packages/node-fetch/src/URL.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ export class PonyfillURL extends FastUrl implements URL {
) {
this.hostname = `[${this.hostname}]`;
}
if (url.includes(`${this.hostname}:`) && !this.port && this.pathname.startsWith('/:')) {
this.port = this.pathname.slice(2);
this.pathname = '/';
this.host = `${this.hostname}:${this.port}`;
}
if (base) {
const baseParsed = typeof base === 'string' ? new PonyfillURL(base) : base;
this.protocol ||= baseParsed.protocol;
Expand All @@ -36,7 +41,7 @@ export class PonyfillURL extends FastUrl implements URL {
}

get origin(): string {
return `${this.protocol}//${this.host}${this.port ? `:${this.port}` : ''}`;
return `${this.protocol}//${this.hostname}${this.port ? `:${this.port}` : ''}`;
}

private _searchParams?: PonyfillURLSearchParams;
Expand Down
38 changes: 38 additions & 0 deletions packages/node-fetch/tests/URL.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,36 @@
import { PonyfillURL } from '../src/URL.js';

describe('URL', () => {
const urlExamples = [
'https://example.com/?foo=bar&bar=qux',
'http://[::1]:8080',
'http://localhost:8080?foo=bar',
];
for (const urlExample of urlExamples) {
describe(urlExample, () => {
it('should parse and stringify URLs', () => {
const url = new PonyfillURL(urlExample);
expect(url.toString()).toBe(urlExample.replace('8080', '8080/'));
});
it('should parse the URLs as expected', () => {
const ponyfillUrl = new PonyfillURL(urlExample);
const nativeUrl = new URL(urlExample);
expect(ponyfillUrl.toString() || null).toBe(nativeUrl.toString() || null);
expect(ponyfillUrl.protocol || null).toBe(nativeUrl.protocol || null);
expect(ponyfillUrl.host || null).toBe(nativeUrl.host || null);
expect(ponyfillUrl.hostname || null).toBe(nativeUrl.hostname || null);
expect(ponyfillUrl.port || null).toBe(nativeUrl.port || null);
expect(ponyfillUrl.pathname || null).toBe(nativeUrl.pathname || null);
expect(ponyfillUrl.search || null).toBe(nativeUrl.search || null);
ponyfillUrl.searchParams.forEach((value, key) => {
expect(nativeUrl.searchParams.get(key) || null).toBe(value || null);
});
expect(ponyfillUrl.username || null).toBe(nativeUrl.username || null);
expect(ponyfillUrl.password || null).toBe(nativeUrl.password || null);
expect(ponyfillUrl.origin || null).toBe(nativeUrl.origin || null);
});
});
}
it('should parse search params', () => {
const url = new PonyfillURL('https://example.com/?foo=bar&foo=baz&bar=qux');
expect(url.searchParams.get('foo')).toBe('bar');
Expand All @@ -22,4 +52,12 @@ describe('URL', () => {
expect(url.hostname).toBe('[::1]');
expect(url.port).toBe('8000');
});
it('parses query strings with ports without pathname', () => {
const url = new PonyfillURL('http://localhost:8080?foo=bar');
expect(url.host).toBe('localhost:8080');
expect(url.hostname).toBe('localhost');
expect(url.pathname).toBe('/');
expect(url.port).toBe('8080');
expect(url.search).toBe('?foo=bar');
});
});
9 changes: 7 additions & 2 deletions packages/server/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,15 @@ app.route({
url: '/mypath',
method: ['GET', 'POST', 'OPTIONS'],
handler: async (req, reply) => {
const response = await myServerAdapter.handleNodeRequestAndResponse(req, reply, {
const response: Response = await myServerAdapter.handleNodeRequestAndResponse(req, reply, {
req,
reply
})

if (!response) {
return reply.status(404).send('Not Found')
}

response.headers.forEach((value, key) => {
reply.header(key, value)
})
Expand Down Expand Up @@ -200,7 +205,7 @@ import myServerAdapter from './myServerAdapter'
const app = new Koa()

app.use(async ctx => {
const response = await myServerAdapter.handleNodeRequest(ctx.req)
const response = await myServerAdapter.handleNodeRequestAndResponse(ctx.request, ctx.res, ctx)

// Set status code
ctx.status = response.status
Expand Down
3 changes: 3 additions & 0 deletions packages/server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,13 @@
"tslib": "^2.6.3"
},
"devDependencies": {
"@hapi/hapi": "^21.3.12",
"@types/express": "5.0.0",
"@types/koa": "^2.15.0",
"@types/node": "22.10.2",
"express": "4.21.2",
"fastify": "5.2.0",
"koa": "^2.15.3",
"react": "19.0.0",
"react-dom": "19.0.0"
},
Expand Down
83 changes: 0 additions & 83 deletions packages/server/test/express.spec.ts

This file was deleted.

Loading

0 comments on commit b82477f

Please sign in to comment.