From 02f30287ee456e132cea053d759ed8ee7665bd82 Mon Sep 17 00:00:00 2001 From: Michael Krasnow Date: Tue, 1 Aug 2023 11:24:32 -0700 Subject: [PATCH 1/3] fix: Allow prefixUrl to work --- src/lib/fetch.ts | 24 ++++++++++++------------ src/test/fetch.request.test.ts | 23 ++++++++++++++--------- src/test/util.ts | 2 +- 3 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/lib/fetch.ts b/src/lib/fetch.ts index 42d1081..bfa384a 100644 --- a/src/lib/fetch.ts +++ b/src/lib/fetch.ts @@ -11,7 +11,16 @@ export function createFetch(got: Got): GotFetch { const globalCache = new Map(); return async (input, opts) => { - const url = new URL(typeof input === 'string' ? input : input.url); + let url = input.toString(); + try { + url = new URL(typeof input === 'string' ? input : input.url).toString(); + } catch (e) { + // Ignore url parsing failure, likely prefixUrl is being used + // When using prefixUrl, the path cannot start with a "/" + if (url.startsWith("/")) { + url = url.substring(1); + } + } const request: RequestInit = typeof input === 'object' ? input : opts || {}; if (request.mode === 'no-cors' || request.mode === 'same-origin' || request.mode === 'navigate') { @@ -31,19 +40,10 @@ export function createFetch(got: Got): GotFetch { throw new TypeError(format('request.headers must be plain object: %j', request.headers)); } - // got does not merge base searchParams with the url's searchParams - // but it does merge searchParams options - // so we clone the url's searchParams - // we also clear the url's search to work around this bug - // https://github.com/sindresorhus/got/issues/1188 - const searchParams = new URLSearchParams(url.searchParams); - url.search = ''; - const gotOpts: GotOptions = { // url needs to be stringified to support UNIX domain sockets, and // For more info see https://github.com/alexghr/got-fetch/pull/8 - url: url.toString(), - searchParams, + url, followRedirect: true, throwHttpErrors: false, method: (request.method as Method) || 'get', @@ -107,7 +107,7 @@ export function createFetch(got: Got): GotFetch { // using Array.prototype.at would've been nice but it's not // supported by anything below Node 16.8 ? r.redirectUrls[r.redirectUrls.length - 1] - : url.href, + : r.url, }); }); } diff --git a/src/test/fetch.request.test.ts b/src/test/fetch.request.test.ts index c2264fa..08c99d3 100644 --- a/src/test/fetch.request.test.ts +++ b/src/test/fetch.request.test.ts @@ -2,7 +2,7 @@ import got from 'got'; import { URLSearchParams } from 'url'; import { createFetch } from '../lib/fetch'; -import { Interceptor, intercept, assert200, url } from './util'; +import { Interceptor, intercept, assert200, url, ORIGIN } from './util'; describe('fetch request', () => { let interceptor: Interceptor; @@ -37,6 +37,18 @@ describe('fetch request', () => { const fetch = createFetch(got); await assert200(fetch(url('/foo'))); }); + + it('allows prefixUrl', async () => { + expect.assertions(1); + interceptor.intercept('/foo', 'get').reply(200); + + const prefixedClient = got.extend({ + prefixUrl: ORIGIN + }) + + const fetch = createFetch(prefixedClient); + await assert200(fetch('/foo')); + }); }); describe('querystring', () => { @@ -48,13 +60,6 @@ describe('fetch request', () => { await assert200(fetch(url('/', { foo: '123', bar: '456' }))); }); - it('merges query string parameters', async () => { - expect.assertions(1); - interceptor.intercept('/', 'get').query({ foo: '123', bar: '456' }).reply(200); - - const fetch = createFetch(got.extend({ searchParams: { bar: '456' } })); - await assert200(fetch(url('/', { foo: '123' }))); - }); }); describe('headers', () => { @@ -175,7 +180,7 @@ describe('fetch request', () => { }); - it.only("sends own content-type header", async () => { + it("sends own content-type header", async () => { expect.assertions(1); // as set by the client diff --git a/src/test/util.ts b/src/test/util.ts index 3d6df84..ddd0dff 100644 --- a/src/test/util.ts +++ b/src/test/util.ts @@ -1,7 +1,7 @@ import nock from 'nock'; import { URL } from 'url'; -const ORIGIN = 'https://example.com'; +export const ORIGIN = 'https://example.com'; export type Interceptor = nock.Scope; export function intercept(): Interceptor { From 96f4c43ac9fdb2bf136e7bc9dc61b330d2c24451 Mon Sep 17 00:00:00 2001 From: Michael Krasnow Date: Tue, 8 Aug 2023 12:51:35 -0700 Subject: [PATCH 2/3] Add back ability to merge queryParams Co-authored-by: Alex Gherghisan --- src/lib/fetch.ts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/lib/fetch.ts b/src/lib/fetch.ts index bfa384a..098d544 100644 --- a/src/lib/fetch.ts +++ b/src/lib/fetch.ts @@ -12,14 +12,31 @@ export function createFetch(got: Got): GotFetch { return async (input, opts) => { let url = input.toString(); + let searchParams: URLSearchParams = new URLSearchParams(); + try { - url = new URL(typeof input === 'string' ? input : input.url).toString(); + const urlObj = new URL( + typeof input === "string" + ? input + : input.url + ); + + searchParams = new URLSearchParams(urlObj.searchParams); + urlObj.search = ""; + + url = urlObj.href; } catch (e) { // Ignore url parsing failure, likely prefixUrl is being used // When using prefixUrl, the path cannot start with a "/" if (url.startsWith("/")) { url = url.substring(1); } + + if (url.includes("?")) { + const [path, search] = url.split("?"); + url = path; + searchParams = new URLSearchParams(search); + } } const request: RequestInit = typeof input === 'object' ? input : opts || {}; From aad8fbdc21b9a402778309bfbff4b5a8be00facd Mon Sep 17 00:00:00 2001 From: Michael Krasnow Date: Tue, 8 Aug 2023 13:10:32 -0700 Subject: [PATCH 3/3] Changes per feedback --- src/lib/fetch.ts | 3 ++- src/test/fetch.request.test.ts | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/lib/fetch.ts b/src/lib/fetch.ts index 098d544..b157039 100644 --- a/src/lib/fetch.ts +++ b/src/lib/fetch.ts @@ -13,7 +13,7 @@ export function createFetch(got: Got): GotFetch { return async (input, opts) => { let url = input.toString(); let searchParams: URLSearchParams = new URLSearchParams(); - + try { const urlObj = new URL( typeof input === "string" @@ -61,6 +61,7 @@ export function createFetch(got: Got): GotFetch { // url needs to be stringified to support UNIX domain sockets, and // For more info see https://github.com/alexghr/got-fetch/pull/8 url, + searchParams, followRedirect: true, throwHttpErrors: false, method: (request.method as Method) || 'get', diff --git a/src/test/fetch.request.test.ts b/src/test/fetch.request.test.ts index 08c99d3..56fccf8 100644 --- a/src/test/fetch.request.test.ts +++ b/src/test/fetch.request.test.ts @@ -62,6 +62,27 @@ describe('fetch request', () => { }); + it('merges query string parameters', async () => { + expect.assertions(1); + interceptor.intercept('/', 'get').query({ foo: '123', bar: '456' }).reply(200); + + const fetch = createFetch(got.extend({ searchParams: { bar: '456' } })); + await assert200(fetch(url('/', { foo: '123' }))); + }); + + it('merges query string parameters with prefixUrl', async () => { + expect.assertions(1); + interceptor.intercept('/foo', 'get').query({ foo: '123', bar: '456' }).reply(200); + + const prefixedClient = got.extend({ + prefixUrl: ORIGIN, + searchParams: { bar: '456' } + }) + + const fetch = createFetch(prefixedClient); + await assert200(fetch('/foo?foo=123')); + }); + describe('headers', () => { it('sends headers', async () => { expect.assertions(1);