fix(ajax): bug fixes and readability improvements

.changeset/eight-pumpkins-clean.md

@@ -0,0 +1,5 @@
+---
+'@lion/ajax': patch
+---
+
+Fix(ajax) options expansion, fix removing request interceptors, use 1 hour as default time to live, check for null when serializing the search params

packages/ajax/src/AjaxClient.js

@@ -27,11 +27,11 @@ export class AjaxClient {
       xsrfCookieName: 'XSRF-TOKEN',
       xsrfHeaderName: 'X-XSRF-TOKEN',
       jsonPrefix: '',
+      ...config,
       cacheOptions: {
         getCacheIdentifier: () => '_default',
         ...config.cacheOptions,
       },
-      ...config,
     };
 
     /** @type {Array.<RequestInterceptor|CachedRequestInterceptor>} */
@@ -43,24 +43,18 @@ export class AjaxClient {
       this.addRequestInterceptor(acceptLanguageRequestInterceptor);
     }
 
-    if (this.__config.xsrfCookieName && this.__config.xsrfHeaderName) {
+    const { xsrfCookieName, xsrfHeaderName } = this.__config;
-      this.addRequestInterceptor(
+    if (xsrfCookieName && xsrfHeaderName) {
-        createXSRFRequestInterceptor(this.__config.xsrfCookieName, this.__config.xsrfHeaderName),
+      this.addRequestInterceptor(createXSRFRequestInterceptor(xsrfCookieName, xsrfHeaderName));
-      );
     }
 
-    if (this.__config.cacheOptions && this.__config.cacheOptions.useCache) {
+    const { cacheOptions } = this.__config;
+    if (cacheOptions?.useCache) {
       this.addRequestInterceptor(
-        cacheRequestInterceptorFactory(
+        cacheRequestInterceptorFactory(cacheOptions.getCacheIdentifier, cacheOptions),
-          this.__config.cacheOptions.getCacheIdentifier,
-          this.__config.cacheOptions,
-        ),
       );
       this.addResponseInterceptor(
-        cacheResponseInterceptorFactory(
+        cacheResponseInterceptorFactory(cacheOptions.getCacheIdentifier, cacheOptions),
-          this.__config.cacheOptions.getCacheIdentifier,
-          this.__config.cacheOptions,
-        ),
       );
     }
   }
@@ -84,10 +78,9 @@ export class AjaxClient {
 
   /** @param {RequestInterceptor} requestInterceptor */
   removeRequestInterceptor(requestInterceptor) {
-    const indexOf = this._requestInterceptors.indexOf(requestInterceptor);
+    this._requestInterceptors = this._requestInterceptors.filter(
-    if (indexOf !== -1) {
+      interceptor => interceptor !== requestInterceptor,
-      this._requestInterceptors.splice(indexOf);
+    );
-    }
   }
 
   /** @param {ResponseInterceptor} responseInterceptor */
@@ -97,10 +90,9 @@ export class AjaxClient {
 
   /** @param {ResponseInterceptor} responseInterceptor */
   removeResponseInterceptor(responseInterceptor) {
-    const indexOf = this._responseInterceptors.indexOf(responseInterceptor);
+    this._responseInterceptors = this._responseInterceptors.filter(
-    if (indexOf !== -1) {
+      interceptor => interceptor !== responseInterceptor,
-      this._responseInterceptors.splice(indexOf, 1);
+    );
-    }
   }
 
   /**
@@ -117,9 +109,9 @@ export class AjaxClient {
     request.params = init?.params;
 
     // run request interceptors, returning directly and skipping the network
-    // if a interceptor returns a Response
     const interceptedRequestOrResponse = await this.__interceptRequest(request);
     if (interceptedRequestOrResponse instanceof Response) {
+      // prevent network request, return cached response
       return interceptedRequestOrResponse;
     }
 
@@ -147,12 +139,12 @@ export class AjaxClient {
     const lionInit = {
       ...init,
       headers: {
-        ...(init && init.headers),
+        ...init?.headers,
         accept: 'application/json',
       },
     };
 
-    if (lionInit && lionInit.body) {
+    if (lionInit?.body) {
       // eslint-disable-next-line no-param-reassign
       lionInit.headers['content-type'] = 'application/json';
       lionInit.body = JSON.stringify(lionInit.body);
@@ -163,10 +155,9 @@ export class AjaxClient {
     const response = await this.request(info, jsonInit);
     let responseText = await response.text();
 
-    if (typeof this.__config.jsonPrefix === 'string') {
+    const { jsonPrefix } = this.__config;
-      if (responseText.startsWith(this.__config.jsonPrefix)) {
+    if (typeof jsonPrefix === 'string' && responseText.startsWith(jsonPrefix)) {
-        responseText = responseText.substring(this.__config.jsonPrefix.length);
+      responseText = responseText.substring(jsonPrefix.length);
-      }
     }
 
     try {

packages/ajax/src/interceptors-cache.js

@@ -6,15 +6,11 @@ import './typedef.js';
 const SECOND = 1000;
 const MINUTE = SECOND * 60;
 const HOUR = MINUTE * 60;
+const DEFAULT_TIME_TO_LIVE = HOUR;
 
 class Cache {
   constructor() {
-    this.expiration = new Date().getTime() + HOUR;
+    this.expiration = new Date().getTime() + DEFAULT_TIME_TO_LIVE;
-    /**
-     * @type {{[url: string]: CacheConfig }}
-     */
-    this.cacheConfig = {};
-
     /**
      * @type {{[url: string]: {expires: number, response: CacheResponse} }}
      * @private
@@ -97,7 +93,7 @@ class Cache {
     this._validateCache();
 
     Object.keys(this._cacheObject).forEach(key => {
-      if (key.indexOf(url) > -1) {
+      if (key.includes(url)) {
         delete this._cacheObject[key];
         this.resolvePendingRequest(key);
       }
@@ -113,15 +109,9 @@ class Cache {
 
     Object.keys(this._cacheObject).forEach(key => {
       const notMatch = !new RegExp(regex).test(key);
-
       if (notMatch) return;
-
+      delete this._cacheObject[key];
-      const isDataDeleted = delete this._cacheObject[key];
       this.resolvePendingRequest(key);
-
-      if (!isDataDeleted) {
-        throw new Error(`Failed to delete cache for a request '${key}'`);
-      }
     });
   }
 
@@ -135,6 +125,7 @@ class Cache {
     if (new Date().getTime() > this.expiration) {
       // @ts-ignore
       this._cacheObject = {};
+      return false;
     }
     return true;
   }
@@ -150,7 +141,7 @@ let caches = {};
  */
 export const searchParamSerializer = (params = {}) =>
   // @ts-ignore
-  typeof params === 'object' ? new URLSearchParams(params).toString() : '';
+  typeof params === 'object' && params !== null ? new URLSearchParams(params).toString() : '';
 
 /**
  * Returns the active cache instance for the current session
@@ -159,10 +150,9 @@ export const searchParamSerializer = (params = {}) =>
  * @param {string} cacheIdentifier usually the refreshToken of the owner of the cache
  */
 const getCache = cacheIdentifier => {
-  if (caches[cacheIdentifier] && caches[cacheIdentifier]._validateCache()) {
+  if (caches[cacheIdentifier]?._validateCache()) {
     return caches[cacheIdentifier];
   }
-
   // invalidate old caches
   caches = {};
   // create new cache
@@ -193,7 +183,7 @@ export const validateOptions = ({
 
   // validate 'timeToLive', default 1 hour
   if (timeToLive === undefined) {
-    timeToLive = 0;
+    timeToLive = DEFAULT_TIME_TO_LIVE;
   }
   if (Number.isNaN(parseInt(String(timeToLive), 10))) {
     throw new Error('Property `timeToLive` must be of type `number`');
@@ -213,7 +203,7 @@ export const validateOptions = ({
   // validate 'requestIdentificationFn', default is url + searchParams
   if (requestIdentificationFn) {
     if (typeof requestIdentificationFn !== 'function') {
-      throw new Error('Property `requestIdentificationFn` must be of type `function` or `falsy`');
+      throw new Error('Property `requestIdentificationFn` must be of type `function`');
     }
   } else {
     requestIdentificationFn = /** @param {any} data */ (
@@ -235,25 +225,6 @@ export const validateOptions = ({
   };
 };
 
-/**
- * Request interceptor to return relevant cached requests
- * @param {ValidatedCacheOptions} validatedInitialCacheOptions
- * @param {CacheOptions=} configCacheOptions
- * @returns {ValidatedCacheOptions}
- */
-function composeCacheOptions(validatedInitialCacheOptions, configCacheOptions) {
-  let actionCacheOptions = validatedInitialCacheOptions;
-
-  if (configCacheOptions) {
-    actionCacheOptions = validateOptions({
-      ...validatedInitialCacheOptions,
-      ...configCacheOptions,
-    });
-  }
-
-  return actionCacheOptions;
-}
-
 /**
  * Request interceptor to return relevant cached requests
  * @param {function(): string} getCacheIdentifier used to invalidate cache if identifier is changed
@@ -264,12 +235,11 @@ export const cacheRequestInterceptorFactory = (getCacheIdentifier, globalCacheOp
   const validatedInitialCacheOptions = validateOptions(globalCacheOptions);
 
   return /** @param {CacheRequest} cacheRequest */ async cacheRequest => {
-    const { method } = cacheRequest;
+    const cacheOptions = validateOptions({
+      ...validatedInitialCacheOptions,
+      ...cacheRequest.cacheOptions,
+    });
 
-    const cacheOptions = composeCacheOptions(
-      validatedInitialCacheOptions,
-      cacheRequest.cacheOptions,
-    );
     cacheRequest.cacheOptions = cacheOptions;
 
     // don't use cache if 'useCache' === false
@@ -278,12 +248,12 @@ export const cacheRequestInterceptorFactory = (getCacheIdentifier, globalCacheOp
     }
 
     const cacheId = cacheOptions.requestIdentificationFn(cacheRequest, searchParamSerializer);
-
     // cacheIdentifier is used to bind the cache to the current session
     const currentCache = getCache(getCacheIdentifier());
+    const { method } = cacheRequest;
 
     // don't use cache if the request method is not part of the configs methods
-    if (cacheOptions.methods.indexOf(method.toLowerCase()) === -1) {
+    if (!cacheOptions.methods.includes(method.toLowerCase())) {
       // If it's NOT one of the config.methods, invalidate caches
       currentCache.delete(cacheId);
       // also invalidate caches matching to cacheOptions
@@ -310,11 +280,7 @@ export const cacheRequestInterceptorFactory = (getCacheIdentifier, globalCacheOp
 
     const cacheResponse = currentCache.get(cacheId, cacheOptions.timeToLive);
     if (cacheResponse) {
-      // eslint-disable-next-line no-param-reassign
+      cacheRequest.cacheOptions = cacheRequest.cacheOptions ?? { useCache: false };
-      if (!cacheRequest.cacheOptions) {
-        cacheRequest.cacheOptions = { useCache: false };
-      }
-
       const response = /** @type {CacheResponse} */ cacheResponse.clone();
       response.request = cacheRequest;
       response.fromCache = true;
@@ -350,10 +316,11 @@ export const cacheResponseInterceptorFactory = (getCacheIdentifier, globalCacheO
       throw new Error('Missing request in response.');
     }
 
-    const cacheOptions = composeCacheOptions(
+    const cacheOptions = validateOptions({
-      validatedInitialCacheOptions,
+      ...validatedInitialCacheOptions,
-      cacheResponse.request?.cacheOptions,
+      ...cacheResponse.request?.cacheOptions,
-    );
+    });
+
     // string that identifies cache entry
     const cacheId = cacheOptions.requestIdentificationFn(
       cacheResponse.request,
@@ -363,13 +330,11 @@ export const cacheResponseInterceptorFactory = (getCacheIdentifier, globalCacheO
     const isAlreadyFromCache = !!cacheResponse.fromCache;
     // caching all responses with not default `timeToLive`
     const isCacheActive = cacheOptions.timeToLive > 0;
-
+    const isMethodSupported = cacheOptions.methods.includes(
+      cacheResponse.request.method.toLowerCase(),
+    );
     // if the request is one of the options.methods; store response in cache
-    if (
+    if (!isAlreadyFromCache && isCacheActive && isMethodSupported) {
-      !isAlreadyFromCache &&
-      isCacheActive &&
-      cacheOptions.methods.indexOf(cacheResponse.request.method.toLowerCase()) > -1
-    ) {
       // store the response data in the cache and mark request as resolved
       currentCache.set(cacheId, cacheResponse.clone());
     }

packages/ajax/src/interceptors.js

@@ -17,10 +17,9 @@ export function getCookie(name, _document = document) {
  */
 export async function acceptLanguageRequestInterceptor(request) {
   if (!request.headers.has('accept-language')) {
-    let locale = document.documentElement.lang || 'en';
+    const documentLocale = document.documentElement.lang;
-    if (document.documentElement.getAttribute('data-localize-lang')) {
+    const localizeLang = document.documentElement.getAttribute('data-localize-lang');
-      locale = document.documentElement.getAttribute('data-localize-lang') || 'en';
+    const locale = localizeLang || documentLocale || 'en';
-    }
     request.headers.set('accept-language', locale);
   }
   return request;

packages/ajax/test/AjaxClient.test.js

@@ -18,6 +18,54 @@ describe('AjaxClient', () => {
     fetchStub.restore();
   });
 
+  describe('options', () => {
+    it('creates options object by expanding cacheOptions', async () => {
+      // Given
+      const getCacheIdentifier = () => '_DEFAULT_CACHE_ID';
+      const config = {
+        jsonPrefix: ")]}',",
+        cacheOptions: {
+          useCache: true,
+          timeToLive: 1000 * 60 * 5, // 5 minutes
+          getCacheIdentifier,
+        },
+      };
+      const expected = {
+        addAcceptLanguage: true,
+        xsrfCookieName: 'XSRF-TOKEN',
+        xsrfHeaderName: 'X-XSRF-TOKEN',
+        jsonPrefix: ")]}',",
+        cacheOptions: {
+          useCache: true,
+          timeToLive: 300000,
+          getCacheIdentifier,
+        },
+      };
+      // When
+      const ajax1 = new AjaxClient(config);
+      const result = ajax1.options;
+      // Then
+      expect(result).to.deep.equal(expected);
+    });
+
+    it('has default getCacheIdentifier function when cacheOptions does not provide one', async () => {
+      // Given
+      const config = {
+        cacheOptions: {
+          useCache: true,
+          timeToLive: 1000 * 60 * 5, // 5 minutes
+        },
+      };
+      // When
+      // @ts-expect-error
+      const ajax1 = new AjaxClient(config);
+      const result = ajax1.options?.cacheOptions?.getCacheIdentifier;
+      // Then
+      expect(result).not.to.be.undefined;
+      expect(result).to.be.a('function');
+    });
+  });
+
   describe('request()', () => {
     it('calls fetch with the given args, returning the result', async () => {
       const response = await (await ajax.request('/foo', { method: 'POST' })).text();
@@ -130,15 +178,22 @@ describe('AjaxClient', () => {
     });
 
     it('removeRequestInterceptor() removes a request interceptor', async () => {
-      const interceptor = /** @param {Request} r */ async r =>
+      const interceptor1 = /** @param {Request} r */ async r =>
         new Request(`${r.url}/intercepted-1`);
-      ajax.addRequestInterceptor(interceptor);
+      const interceptor2 = /** @param {Request} r */ async r =>
-      ajax.removeRequestInterceptor(interceptor);
+        new Request(`${r.url}/intercepted-2`);
+      const interceptor3 = /** @param {Request} r */ async r =>
+        new Request(`${r.url}/intercepted-3`);
+
+      ajax.addRequestInterceptor(interceptor1);
+      ajax.addRequestInterceptor(interceptor2);
+      ajax.addRequestInterceptor(interceptor3);
+      ajax.removeRequestInterceptor(interceptor1);
 
       await ajax.request('/foo', { method: 'POST' });
 
       const request = fetchStub.getCall(0).args[0];
-      expect(request.url).to.equal(`${window.location.origin}/foo`);
+      expect(request.url).to.equal(`${window.location.origin}/foo/intercepted-2/intercepted-3`);
     });
 
     it('removeResponseInterceptor() removes a request interceptor', async () => {
@@ -148,8 +203,9 @@ describe('AjaxClient', () => {
       // @ts-expect-error we're mocking the response as a simple promise which returns a string
       ajax.removeResponseInterceptor(interceptor);
 
-      const response = await (await ajax.request('/foo', { method: 'POST' })).text();
+      const response = await ajax.request('/foo', { method: 'POST' });
-      expect(response).to.equal('mock response');
+      const text = await response.text();
+      expect(text).to.equal('mock response');
     });
   });
 

packages/ajax/test/interceptors-cache.test.js

@@ -135,7 +135,7 @@ describe('ajax cache', () => {
           requestIdentificationFn: 'not a function',
         });
         removeCacheInterceptors(ajax, indexes);
-      }).to.throw(/Property `requestIdentificationFn` must be of type `function` or `falsy`/);
+      }).to.throw(/Property `requestIdentificationFn` must be of type `function`/);
     });
   });