fix: fixed fetch issues

- fixed issue with fetchURL() where no relative URL could be returned as attempting to create a new URL with a relative path and no base would throw an error
- fixed issue where any paths on the base url were not being retained when creating the final url
- added fixExtraQueryParameters()
- added additional tests

packages/util/fetch/src/lib/fetch.spec.ts

@@ -32,34 +32,72 @@ describe('fetchRequestFactory()', () => {
       });
 
       describe('valid', () => {
-        const baseUrl = 'https://components.dereekb.com/';
+        describe('base url as domain and path', () => {
+          describe('with slash at end', () => {
+            const baseUrl = 'https://components.dereekb.com/api/';
 
-        const factory = testFetch.fetchRequestFactory({
-          baseUrl
-        });
+            const factory = testFetch.fetchRequestFactory({
+              baseUrl
+            });
 
-        it('should retain the path of an input request.', () => {
-          const expectedUrl = 'https://google.com/';
-          const request = testFetch.makeRequest(expectedUrl);
-          const result = factory(request);
-          expect(result.url).toBe(expectedUrl);
-        });
+            describe('url input as string', () => {
+              it('should append the base url with the path to the request.', () => {
+                const result = factory('test');
+                expect(result.url).toBe(`${baseUrl}test`);
+              });
+            });
+          });
 
-        it('should retain the path of an input URL.', () => {
-          const expectedUrl = 'https://google.com/';
-          const request = new URL(expectedUrl);
-          const result = factory(request);
-          expect(result.url).toBe(expectedUrl);
-        });
+          describe('without slash at end', () => {
+            const baseUrl = 'https://components.dereekb.com/api';
+
+            const factory = testFetch.fetchRequestFactory({
+              baseUrl
+            });
 
-        it('should append the base url to the request.', () => {
-          const result = factory('test');
-          expect(result.url).toBe('https://components.dereekb.com/test');
+            describe('url input as string', () => {
+              it('should append the base url with the path to the request.', () => {
+                const result = factory('test');
+                expect(result.url).toBe(`${baseUrl}/test`);
+              });
+            });
+          });
         });
 
-        it('should append the base url to the request if it has a front slash.', () => {
-          const result = factory('/test');
-          expect(result.url).toBe('https://components.dereekb.com/test');
+        describe('base url as domain', () => {
+          const baseUrl = 'https://components.dereekb.com/';
+
+          const factory = testFetch.fetchRequestFactory({
+            baseUrl
+          });
+
+          describe('url input as string', () => {
+            it('should retain the path of an input request if it begins with http(s).', () => {
+              const expectedUrl = 'https://google.com/';
+              const request = testFetch.makeRequest(expectedUrl);
+              const result = factory(request);
+              expect(result.url).toBe(expectedUrl);
+            });
+
+            it('should append the base url to the request.', () => {
+              const result = factory('test');
+              expect(result.url).toBe('https://components.dereekb.com/test');
+            });
+
+            it('should append the base url to the request if it has a front slash.', () => {
+              const result = factory('/test');
+              expect(result.url).toBe('https://components.dereekb.com/test');
+            });
+          });
+
+          describe('url input as URL', () => {
+            it('should use the URL as is, and ignore the baseUrl.', () => {
+              const expectedUrl = 'https://google.com/';
+              const request = new URL(expectedUrl);
+              const result = factory(request);
+              expect(result.url).toBe(expectedUrl);
+            });
+          });
         });
       });
     });

packages/util/fetch/src/lib/fetch.ts

@@ -1,4 +1,4 @@
-import { Factory, MapFunction, Maybe, removeTrailingFileTypeSeparators, WebsitePath, WebsiteUrl } from '@dereekb/util';
+import { Factory, fixMultiSlashesInSlashPath, MapFunction, Maybe, removeTrailingFileTypeSeparators, removeTrailingSlashes, WebsitePath, WebsiteUrl } from '@dereekb/util';
 import { fetchOk, requireOkResponse } from './error';
 import { ConfiguredFetchWithTimeout, RequestInitWithTimeout, RequestWithTimeout } from './fetch.type';
 import { fetchTimeout } from './timeout';
@@ -132,12 +132,15 @@ export type AbortControllerFactory = Factory<AbortController>;
 
 export function fetchRequestFactory(config: FetchRequestFactoryInput): FetchRequestFactory {
   const { makeRequest = (input, init) => new Request(input, init), baseUrl: inputBaseUrl, baseRequest: inputBaseRequest, timeout, requestInitFactory, useBaseUrlForConfiguredFetchRequests = false } = config;
-  const baseUrl = inputBaseUrl ? new URL(removeTrailingFileTypeSeparators(inputBaseUrl)) : undefined;
+  const baseUrl = inputBaseUrl ? new URL(removeTrailingSlashes(inputBaseUrl)) : undefined;
   const baseRequest = timeout ? { ...inputBaseRequest, timeout } : inputBaseRequest;
 
   const buildUrl = baseUrl
     ? (url: string | WebsitePath | URL) => {
-        return new URL(url, baseUrl);
+        // retain the origin and any pathname from the base url
+        const urlPath = baseUrl.origin + fixMultiSlashesInSlashPath('/' + baseUrl.pathname + '/' + url);
+        const result = new URL(urlPath, baseUrl);
+        return result;
       }
     : undefined;
 

packages/util/fetch/src/lib/json.spec.ts

@@ -1,4 +1,4 @@
-import { ServerError } from '@dereekb/util';
+import { ServerError, WebsiteDomain, WebsitePath } from '@dereekb/util';
 import { itShouldFail, expectFail, failDueToSuccess, failSuccessfully } from '@dereekb/util/test';
 import { fetchService, fetchRequestFactory, FetchRequestFactory, FetchService } from './fetch';
 import fetch, { Request, RequestInfo, RequestInit } from 'node-fetch';
@@ -12,11 +12,28 @@ jest.setTimeout(30000);
 
 describe('fetchJson()', () => {
   // Expected result: {"statusCode":403,"message":"Forbidden"}
+  const forbiddenUrlBaseUrl: WebsiteDomain = 'https://components.dereekb.com/api';
+  const forbiddernUrlRelativeUrl: WebsitePath = '/webhook';
+
   const forbiddenUrl = 'https://components.dereekb.com/api/webhook';
 
   const fetch = testFetch.makeFetch();
   const fetchJson = fetchJsonFunction(fetch);
 
+  describe('fetch with base url', () => {
+    const fetch = testFetch.makeFetch({
+      baseUrl: forbiddenUrlBaseUrl
+    });
+
+    const fetchJson = fetchJsonFunction(fetch);
+
+    it('should send a GET request by default.', async () => {
+      // NOTE: Fetch will not throw an error on non-ok results, allowing us to test against the webhook url.
+      const response = await fetchJson<{ statusCode: number; message: 'Forbidden' }>(forbiddernUrlRelativeUrl);
+      expect(response.message).toBe('Forbidden');
+    });
+  });
+
   describe('GET', () => {
     const method = 'GET';
 

packages/util/fetch/src/lib/url.spec.ts

@@ -42,26 +42,26 @@ describe('fetchURL()', () => {
   describe('with string url', () => {
     it('should return the url', () => {
       const result = fetchURL(urlString);
-      expect(result.href).toBe(urlString);
+      expect(result).toBe(urlString);
     });
   });
 
   describe('with URL', () => {
     it('should return the url', () => {
       const result = fetchURL(url);
-      expect(result.href).toBe(url.href);
+      expect(result).toBe(url.href);
     });
   });
 
   describe('with FetchURLConfiguration input', () => {
     it('should return the url', () => {
       const result = fetchURL({ url });
-      expect(result.href).toBe(url.href);
+      expect(result).toBe(url.href);
     });
 
     it('should return the url with query params attached', () => {
       const result = fetchURL({ url, queryParams });
-      expect(result.href).toBe(url.href + `?${queryParams.toString()}`);
+      expect(result).toBe(url.href + `?${queryParams.toString()}`);
     });
 
     it('should return the url with query params attached as an object', () => {
@@ -71,7 +71,7 @@ describe('fetchURL()', () => {
       };
       const expectedParams = objectToMap(queryParamsObject);
       const result = fetchURL({ url, queryParams: queryParamsObject });
-      expect(result.href).toBe(url.href + `?${new URLSearchParams(Array.from(expectedParams.entries())).toString()}`);
+      expect(result).toBe(url.href + `?${new URLSearchParams(Array.from(expectedParams.entries())).toString()}`);
     });
 
     it('should return the url with query params of different types attached as an object', () => {
@@ -82,7 +82,7 @@ describe('fetchURL()', () => {
       };
       const expectedParams = objectToMap(queryParamsObject);
       const result = fetchURL({ url, queryParams: queryParamsObject });
-      expect(result.href).toBe(url.href + `?${new URLSearchParams(Array.from(expectedParams.entries()).map((x) => [String(x[0]), String(x[1])])).toString()}`);
+      expect(result).toBe(url.href + `?${new URLSearchParams(Array.from(expectedParams.entries()).map((x) => [String(x[0]), String(x[1])])).toString()}`);
     });
 
     it('should return the url with query params of different types attached as an object with an array as a set of values for a key', () => {
@@ -92,7 +92,7 @@ describe('fetchURL()', () => {
       };
 
       const result = fetchURL({ url, queryParams: queryParamsObject });
-      expect(result.href).toBe(url.href + `?${queryParams.toString()}`);
+      expect(result).toBe(url.href + `?${queryParams.toString()}`);
     });
 
     it('should return the url with query params of different types attached as an object and ignore null values', () => {
@@ -104,23 +104,23 @@ describe('fetchURL()', () => {
       };
 
       const result = fetchURL({ url, queryParams: queryParamsObject });
-      expect(result.href).toBe(url.href + `?${queryParams.toString()}`);
+      expect(result).toBe(url.href + `?${queryParams.toString()}`);
     });
 
     it('should return the url with query params attached as a tuple', () => {
       const result = fetchURL({ url, queryParams: queryParamsTuples });
-      expect(result.href).toBe(url.href + `?${queryParams.toString()}`);
+      expect(result).toBe(url.href + `?${queryParams.toString()}`);
     });
 
     it('should return the url with query params attached as a tuple with an array of values', () => {
       const result = fetchURL({ url, queryParams: queryParamsTuples });
-      expect(result.href).toBe(url.href + `?${queryParams.toString()}`);
+      expect(result).toBe(url.href + `?${queryParams.toString()}`);
     });
 
     it('should return the url with query params attached as a Map', () => {
       const expectedParams = new Map(queryParamsTuples);
       const result = fetchURL({ url, queryParams: expectedParams });
-      expect(result.href).toBe(url.href + `?${new URLSearchParams(Array.from(expectedParams.entries())).toString()}`);
+      expect(result).toBe(url.href + `?${new URLSearchParams(Array.from(expectedParams.entries())).toString()}`);
     });
 
     it('should return the url with query params attached as a tuples array', () => {
@@ -131,7 +131,7 @@ describe('fetchURL()', () => {
       ];
 
       const result = fetchURL({ url, queryParams: queryParamsTuplesWithArray });
-      expect(result.href).toBe(url.href + `?${queryParams.toString()}`);
+      expect(result).toBe(url.href + `?${queryParams.toString()}`);
     });
 
     it('should return the url with query params attached as a tuples array undefined keys and values', () => {
@@ -143,17 +143,17 @@ describe('fetchURL()', () => {
       ];
 
       const result = fetchURL({ url, queryParams: queryParamsTuplesWithArray });
-      expect(result.href).toBe(url.href + `?${queryParams.toString()}`);
+      expect(result).toBe(url.href + `?${queryParams.toString()}`);
     });
 
     it('should return the url with query params attached as a string with a "?" attached', () => {
       const result = fetchURL({ url, queryParams: `?${queryParams.toString()}` });
-      expect(result.href).toBe(url.href + `?${queryParams.toString()}`);
+      expect(result).toBe(url.href + `?${queryParams.toString()}`);
     });
 
     it('should return the url with query params attached as a string without a "?" attached', () => {
       const result = fetchURL({ url, queryParams: `${queryParams.toString()}` });
-      expect(result.href).toBe(url.href + `?${queryParams.toString()}`);
+      expect(result).toBe(url.href + `?${queryParams.toString()}`);
     });
   });
 });

packages/util/fetch/src/lib/url.ts

@@ -1,4 +1,4 @@
-import { ArrayOrValue, forEachInIterable, forEachKeyValue, isIterable, mapIterable, Maybe, useIterableOrValue } from '@dereekb/util';
+import { ArrayOrValue, fixExtraQueryParameters, forEachInIterable, forEachKeyValue, isIterable, mapIterable, Maybe, useIterableOrValue } from '@dereekb/util';
 
 export type SimpleFetchURLInput = URL | string;
 
@@ -15,19 +15,19 @@ export interface FetchURLConfiguration {
 
 export type FetchURLInput = SimpleFetchURLInput | FetchURLConfiguration;
 
-export function fetchURL(input: FetchURLInput): URL {
-  let url: URL;
+export function fetchURL(input: FetchURLInput): string {
+  let url: string;
 
   if (typeof input === 'string') {
-    url = new URL(input);
-  } else if (isURL(input)) {
     url = input;
+  } else if (isURL(input)) {
+    url = input.href;
   } else {
     const baseUrl = fetchURL(input.url);
 
     if (input.queryParams) {
       const searchParams = queryParamsToSearchParams(input.queryParams);
-      url = new URL(`?${searchParams.toString()}`, baseUrl);
+      url = fixExtraQueryParameters(baseUrl + `?${searchParams.toString()}`);
     } else {
       url = baseUrl;
     }

packages/util/jest.config.ts

@@ -12,5 +12,6 @@ module.exports = {
     '^.+\\.[tj]sx?$': 'ts-jest'
   },
   moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx'],
+  modulePathIgnorePatterns: ['test', 'fetch'],
   coverageDirectory: '../../coverage/packages/util'
 };

packages/util/src/lib/path/path.spec.ts

@@ -1,4 +1,4 @@
-import { replaceInvalidFilePathTypeSeparatorsInSlashPath, slashPathFactory, slashPathName, slashPathValidationFactory, SlashPathFolder, slashPathType, SlashPathTypedFile, SlashPathFile, SLASH_PATH_SEPARATOR, isolateSlashPathFunction } from './path';
+import { replaceInvalidFilePathTypeSeparatorsInSlashPath, slashPathFactory, slashPathName, slashPathValidationFactory, SlashPathFolder, slashPathType, SlashPathTypedFile, SlashPathFile, SLASH_PATH_SEPARATOR, isolateSlashPathFunction, removeTrailingFileTypeSeparators, removeTrailingSlashes } from './path';
 
 describe('slashPathName', () => {
   it('should return the file name', () => {
@@ -40,6 +40,40 @@ describe('slashPathType', () => {
   });
 });
 
+describe('removeTrailingSlashes()', () => {
+  it('should remove all trailing slashes', () => {
+    const folderName = 'wMNzlhSlp6Gb93V8u4Rs';
+    const folderPath: SlashPathFolder = `${folderName}/`;
+    const result = removeTrailingSlashes(folderPath);
+    expect(result).toBe(folderName);
+  });
+
+  it('should remove all trailing slashes from a url', () => {
+    const url = 'https://components.dereekb.com';
+    const urlPath = `${url}/`;
+    const result = removeTrailingSlashes(urlPath);
+    expect(result).toBe(url);
+  });
+});
+
+describe('removeTrailingFileTypeSeparators()', () => {
+  it('should remove the trailing file type separator (".")', () => {
+    const filename = 'filename';
+    const typedFilePath: SlashPathTypedFile = `${filename}.`;
+
+    const result = removeTrailingFileTypeSeparators(typedFilePath);
+    expect(result).toBe(filename);
+  });
+
+  it('should not remove the trailing file type.', () => {
+    const filename = 'filename';
+    const typedFilePath: SlashPathTypedFile = `${filename}.pdf`;
+
+    const result = removeTrailingFileTypeSeparators(typedFilePath);
+    expect(result).toBe(typedFilePath);
+  });
+});
+
 describe('replaceInvalidFilePathTypeSeparatorsInSlashPath()', () => {
   it('should replace all extra file path type separators', () => {
     const value = '/path/to/file.who.thought.about.it.test.png';

packages/util/src/lib/path/path.ts

@@ -186,6 +186,10 @@ export function replaceMultipleFilePathsInSlashPath(input: SlashPath): SlashPath
   return input.replace(ALL_DOUBLE_SLASHES_REGEX, SLASH_PATH_SEPARATOR);
 }
 
+export function removeTrailingSlashes(input: SlashPath): SlashPath {
+  return input.replace(TRAILING_SLASHES_REGEX, '');
+}
+
 export function removeTrailingFileTypeSeparators(input: SlashPath): SlashPath {
   return input.replace(TRAILING_FILE_TYPE_SEPARATORS_REGEX, '');
 }