@lion/ajax - npm Package Compare versions

Comparing version 0.8.0 to 0.9.0

CHANGELOG.md

 # Change Log
 
+## 0.9.0
+
+### Minor Changes
+
+- 43e4bb81: Type fixes and enhancements:
+
+  - all protected/private entries added to form-core type definitions, and their dependents were fixed
+  - a lot @ts-expect-error and @ts-ignore (all `get slots()` and `get modelValue()` issues are fixed)
+  - categorized @ts-expect-error / @ts-ignore into:
+    - [external]: when a 3rd party didn't ship types (could also be browser specs)
+    - [allow-protected]: when we are allowed to know about protected methods. For instance when code
+      resides in the same package
+    - [allow-private]: when we need to check a private value inside a test
+    - [allow]: miscellaneous allows
+    - [editor]: when the editor complains, but the cli/ci doesn't
+
+### Patch Changes
+
+- c1a81fe4: allow caching concurrent requests
+- 9b79b287: Fix(ajax) options expansion, fix removing request interceptors, use 1 hour as default time to live, check for null when serializing the search params
+- 77a04245: add protected and private type info
+- 468223a0: return cached status and headers
+
 ## 0.8.0
@@ -4,0 +27,0 @@

package.json

 {
   "name": "@lion/ajax",
-  "version": "0.8.0",
+  "version": "0.9.0",
   "description": "Thin wrapper around fetch with support for interceptors.",
@@ -5,0 +5,0 @@ "license": "MIT",

src/AjaxClient.d.ts

@@ -11,4 +11,7 @@ /**
     constructor(config?: Partial<AjaxClientConfig>);
-    /** @type {Partial<AjaxClientConfig>} */
-    __config: Partial<AjaxClientConfig>;
+    /**
+     * @type {Partial<AjaxClientConfig>}
+     * @private
+     */
+    private __config;
     /** @type {Array.<RequestInterceptor|CachedRequestInterceptor>} */
@@ -54,2 +57,12 @@ _requestInterceptors: Array<RequestInterceptor | CachedRequestInterceptor>;
     }>;
+    /**
+     * @param {Request} request
+     * @returns {Promise<Request | Response>}
+     */
+    __interceptRequest(request: Request): Promise<Request | Response>;
+    /**
+     * @param {Response} response
+     * @returns {Promise<Response>}
+     */
+    __interceptResponse(response: Response): Promise<Response>;
 }

src/AjaxClient.js

@@ -21,3 +21,6 @@ /* eslint-disable consistent-return */
   constructor(config = {}) {
-    /** @type {Partial<AjaxClientConfig>} */
+    /**
+     * @type {Partial<AjaxClientConfig>}
+     * @private
+     */
     this.__config = {
@@ -28,2 +31,3 @@ addAcceptLanguage: true,
       jsonPrefix: '',
+      ...config,
       cacheOptions: {
@@ -33,3 +37,2 @@ getCacheIdentifier: () => '_default',
       },
-      ...config,
     };
@@ -46,20 +49,14 @@
 
-    if (this.__config.xsrfCookieName && this.__config.xsrfHeaderName) {
-      this.addRequestInterceptor(
-        createXSRFRequestInterceptor(this.__config.xsrfCookieName, this.__config.xsrfHeaderName),
-      );
+    const { xsrfCookieName, xsrfHeaderName } = this.__config;
+    if (xsrfCookieName && xsrfHeaderName) {
+      this.addRequestInterceptor(createXSRFRequestInterceptor(xsrfCookieName, xsrfHeaderName));
     }
 
-    if (this.__config.cacheOptions && this.__config.cacheOptions.useCache) {
+    const { cacheOptions } = this.__config;
+    if (cacheOptions?.useCache) {
       this.addRequestInterceptor(
-        cacheRequestInterceptorFactory(
-          this.__config.cacheOptions.getCacheIdentifier,
-          this.__config.cacheOptions,
-        ),
+        cacheRequestInterceptorFactory(cacheOptions.getCacheIdentifier, cacheOptions),
       );
       this.addResponseInterceptor(
-        cacheResponseInterceptorFactory(
-          this.__config.cacheOptions.getCacheIdentifier,
-          this.__config.cacheOptions,
-        ),
+        cacheResponseInterceptorFactory(cacheOptions.getCacheIdentifier, cacheOptions),
       );
@@ -88,6 +85,5 @@ }
   removeRequestInterceptor(requestInterceptor) {
-    const indexOf = this._requestInterceptors.indexOf(requestInterceptor);
-    if (indexOf !== -1) {
-      this._requestInterceptors.splice(indexOf);
-    }
+    this._requestInterceptors = this._requestInterceptors.filter(
+      interceptor => interceptor !== requestInterceptor,
+    );
   }
@@ -102,6 +98,5 @@
   removeResponseInterceptor(responseInterceptor) {
-    const indexOf = this._responseInterceptors.indexOf(responseInterceptor);
-    if (indexOf !== -1) {
-      this._responseInterceptors.splice(indexOf, 1);
-    }
+    this._responseInterceptors = this._responseInterceptors.filter(
+      interceptor => interceptor !== responseInterceptor,
+    );
   }
@@ -123,24 +118,12 @@
     // run request interceptors, returning directly and skipping the network
-    // if a interceptor returns a Response
-    let interceptedRequest = request;
-    for (const intercept of this._requestInterceptors) {
-      // In this instance we actually do want to await for each sequence
-      // eslint-disable-next-line no-await-in-loop
-      const interceptedRequestOrResponse = await intercept(interceptedRequest);
-      if (interceptedRequestOrResponse instanceof Request) {
-        interceptedRequest = interceptedRequestOrResponse;
-      } else {
-        return interceptedRequestOrResponse;
-      }
+    const interceptedRequestOrResponse = await this.__interceptRequest(request);
+    if (interceptedRequestOrResponse instanceof Response) {
+      // prevent network request, return cached response
+      return interceptedRequestOrResponse;
     }
 
-    const response = /** @type {CacheResponse} */ (await fetch(interceptedRequest));
-    response.request = interceptedRequest;
+    const response = /** @type {CacheResponse} */ (await fetch(interceptedRequestOrResponse));
+    response.request = interceptedRequestOrResponse;
 
-    let interceptedResponse = response;
-    for (const intercept of this._responseInterceptors) {
-      // In this instance we actually do want to await for each sequence
-      // eslint-disable-next-line no-await-in-loop
-      interceptedResponse = await intercept(interceptedResponse);
-    }
+    const interceptedResponse = await this.__interceptResponse(response);
 
@@ -166,3 +149,3 @@ if (interceptedResponse.status >= 400 && interceptedResponse.status < 600) {
       headers: {
-        ...(init && init.headers),
+        ...init?.headers,
         accept: 'application/json',
@@ -172,3 +155,3 @@ },
 
-    if (lionInit && lionInit.body) {
+    if (lionInit?.body) {
       // eslint-disable-next-line no-param-reassign
@@ -184,6 +167,5 @@ lionInit.headers['content-type'] = 'application/json';
 
-    if (typeof this.__config.jsonPrefix === 'string') {
-      if (responseText.startsWith(this.__config.jsonPrefix)) {
-        responseText = responseText.substring(this.__config.jsonPrefix.length);
-      }
+    const { jsonPrefix } = this.__config;
+    if (typeof jsonPrefix === 'string' && responseText.startsWith(jsonPrefix)) {
+      responseText = responseText.substring(jsonPrefix.length);
     }
@@ -200,2 +182,37 @@
   }
+
+  /**
+   * @param {Request} request
+   * @returns {Promise<Request | Response>}
+   */
+  async __interceptRequest(request) {
+    // run request interceptors, returning directly and skipping the network
+    // if a interceptor returns a Response
+    let interceptedRequest = request;
+    for (const intercept of this._requestInterceptors) {
+      // In this instance we actually do want to await for each sequence
+      // eslint-disable-next-line no-await-in-loop
+      const interceptedRequestOrResponse = await intercept(interceptedRequest);
+      if (interceptedRequestOrResponse instanceof Request) {
+        interceptedRequest = interceptedRequestOrResponse;
+      } else {
+        return this.__interceptResponse(interceptedRequestOrResponse);
+      }
+    }
+    return interceptedRequest;
+  }
+
+  /**
+   * @param {Response} response
+   * @returns {Promise<Response>}
+   */
+  async __interceptResponse(response) {
+    let interceptedResponse = response;
+    for (const intercept of this._responseInterceptors) {
+      // In this instance we actually do want to await for each sequence
+      // eslint-disable-next-line no-await-in-loop
+      interceptedResponse = await intercept(interceptedResponse);
+    }
+    return interceptedResponse;
+  }
 }

src/interceptors-cache.js

@@ -9,27 +9,57 @@ /* eslint-disable consistent-return */
 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 }}
+     * @type {{[url: string]: {expires: number, response: CacheResponse} }}
+     * @private
      */
-    this.cacheConfig = {};
-
+    this._cacheObject = {};
     /**
-     * @type {{[url: string]: {expires: number, data: object} }}
+     * @type {{ [url: string]: { promise: Promise<void>, resolve: (v?: any) => void } }}
+     * @private
      */
-    this._cacheObject = {};
+    this._pendingRequests = {};
   }
 
+  /** @param {string} url  */
+  setPendingRequest(url) {
+    /** @type {(v: any) => void} */
+    let resolve = () => {};
+    const promise = new Promise(_resolve => {
+      resolve = _resolve;
+    });
+    this._pendingRequests[url] = { promise, resolve };
+  }
+
   /**
+   * @param {string} url
+   * @returns {Promise<void> | undefined}
+   */
+  getPendingRequest(url) {
+    if (this._pendingRequests[url]) {
+      return this._pendingRequests[url].promise;
+    }
+  }
+
+  /** @param {string} url */
+  resolvePendingRequest(url) {
+    if (this._pendingRequests[url]) {
+      this._pendingRequests[url].resolve();
+      delete this._pendingRequests[url];
+    }
+  }
+
+  /**
    * Store an item in the cache
    * @param {string} url key by which the cache is stored
-   * @param {object} data the cached object
+   * @param {Response} response the cached response
    */
-  set(url, data) {
+  set(url, response) {
     this._validateCache();
     this._cacheObject[url] = {
       expires: new Date().getTime(),
-      data,
+      response,
     };
@@ -42,2 +72,3 @@ }
    * @param {number} timeToLive maximum time to allow cache to live
+   * @returns {CacheResponse | false}
    */
@@ -56,3 +87,3 @@ get(url, timeToLive) {
     }
-    return cacheResult.data;
+    return cacheResult.response;
   }
@@ -68,4 +99,5 @@
     Object.keys(this._cacheObject).forEach(key => {
-      if (key.indexOf(url) > -1) {
+      if (key.includes(url)) {
         delete this._cacheObject[key];
+        this.resolvePendingRequest(key);
       }
@@ -84,10 +116,5 @@ });
       const notMatch = !new RegExp(regex).test(key);
-
       if (notMatch) return;
-
-      const isDataDeleted = delete this._cacheObject[key];
-
-      if (!isDataDeleted) {
-        throw new Error(`Failed to delete cache for a request '${key}'`);
-      }
+      delete this._cacheObject[key];
+      this.resolvePendingRequest(key);
     });
@@ -100,7 +127,8 @@ }
    * empty object
+   * @protected
    */
   _validateCache() {
     if (new Date().getTime() > this.expiration) {
-      // @ts-ignore
       this._cacheObject = {};
+      return false;
     }
@@ -120,4 +148,3 @@ return true;
 export const searchParamSerializer = (params = {}) =>
-  // @ts-ignore
-  typeof params === 'object' ? new URLSearchParams(params).toString() : '';
+  typeof params === 'object' && params !== null ? new URLSearchParams(params).toString() : '';
 
@@ -131,6 +158,5 @@ /**
 const getCache = cacheIdentifier => {
-  if (caches[cacheIdentifier] && caches[cacheIdentifier]._validateCache()) {
+  if (caches[cacheIdentifier]?._validateCache()) {
     return caches[cacheIdentifier];
   }
-
   // invalidate old caches
@@ -166,3 +192,3 @@ caches = {};
   if (timeToLive === undefined) {
-    timeToLive = 0;
+    timeToLive = DEFAULT_TIME_TO_LIVE;
   }
@@ -187,3 +213,3 @@ if (Number.isNaN(parseInt(String(timeToLive), 10))) {
     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`');
     }
@@ -212,21 +238,2 @@ } else {
  * 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
@@ -240,8 +247,7 @@ * @param {CacheOptions} globalCacheOptions
   return /** @param {CacheRequest} cacheRequest */ async cacheRequest => {
-    const { method, status, statusText, headers } = cacheRequest;
+    const cacheOptions = validateOptions({
+      ...validatedInitialCacheOptions,
+      ...cacheRequest.cacheOptions,
+    });
 
-    const cacheOptions = composeCacheOptions(
-      validatedInitialCacheOptions,
-      cacheRequest.cacheOptions,
-    );
     cacheRequest.cacheOptions = cacheOptions;
@@ -255,9 +261,8 @@
     const cacheId = cacheOptions.requestIdentificationFn(cacheRequest, searchParamSerializer);
-
     // cacheIdentifier is used to bind the cache to the current session
     const currentCache = getCache(getCacheIdentifier());
-    const cacheResponse = currentCache.get(cacheId, cacheOptions.timeToLive);
+    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
@@ -281,15 +286,12 @@ currentCache.delete(cacheId);
 
+    const pendingRequest = currentCache.getPendingRequest(cacheId);
+    if (pendingRequest) {
+      // there is another concurrent request, wait for it to finish
+      await pendingRequest;
+    }
+
+    const cacheResponse = currentCache.get(cacheId, cacheOptions.timeToLive);
     if (cacheResponse) {
-      // eslint-disable-next-line no-param-reassign
-      if (!cacheRequest.cacheOptions) {
-        cacheRequest.cacheOptions = { useCache: false };
-      }
-
-      const init = /** @type {LionRequestInit} */ ({
-        status,
-        statusText,
-        headers,
-      });
-
-      const response = /** @type {CacheResponse} */ (new Response(cacheResponse, init));
+      cacheRequest.cacheOptions = cacheRequest.cacheOptions ?? { useCache: false };
+      const response = /** @type {CacheResponse} */ cacheResponse.clone();
       response.request = cacheRequest;
@@ -300,2 +302,6 @@ response.fromCache = true;
 
+    // we do want to use caching for this requesting, but it's not already cached
+    // mark this as a pending request, so that concurrent requests can reuse it from the cache
+    currentCache.setPendingRequest(cacheId);
+
     return cacheRequest;
@@ -322,36 +328,32 @@ };
 
-    const cacheOptions = composeCacheOptions(
-      validatedInitialCacheOptions,
-      cacheResponse.request?.cacheOptions,
+    if (!cacheResponse.request) {
+      throw new Error('Missing request in response.');
+    }
+
+    const cacheOptions = validateOptions({
+      ...validatedInitialCacheOptions,
+      ...cacheResponse.request?.cacheOptions,
+    });
+
+    // string that identifies cache entry
+    const cacheId = cacheOptions.requestIdentificationFn(
+      cacheResponse.request,
+      searchParamSerializer,
     );
-
+    const currentCache = getCache(getCacheIdentifier());
     const isAlreadyFromCache = !!cacheResponse.fromCache;
     // caching all responses with not default `timeToLive`
     const isCacheActive = cacheOptions.timeToLive > 0;
-
-    if (isAlreadyFromCache || !isCacheActive) {
-      return cacheResponse;
-    }
-
+    const isMethodSupported = cacheOptions.methods.includes(
+      cacheResponse.request.method.toLowerCase(),
+    );
     // if the request is one of the options.methods; store response in cache
-    if (
-      cacheResponse.request &&
-      cacheOptions.methods.indexOf(cacheResponse.request.method.toLowerCase()) > -1
-    ) {
-      // string that identifies cache entry
-      const cacheId = cacheOptions.requestIdentificationFn(
-        cacheResponse.request,
-        searchParamSerializer,
-      );
-
-      const responseBody = await cacheResponse.clone().text();
-      // store the response data in the cache
-      getCache(getCacheIdentifier()).set(cacheId, responseBody);
-    } else {
-      // don't store in cache if the request method is not part of the configs methods
-      return cacheResponse;
+    if (!isAlreadyFromCache && isCacheActive && isMethodSupported) {
+      // store the response data in the cache and mark request as resolved
+      currentCache.set(cacheId, cacheResponse.clone());
     }
 
+    currentCache.resolvePendingRequest(cacheId);
     return cacheResponse;
   };
 };

src/interceptors.js

@@ -20,6 +20,5 @@ import './typedef.js';
   if (!request.headers.has('accept-language')) {
-    let locale = document.documentElement.lang || 'en';
-    if (document.documentElement.getAttribute('data-localize-lang')) {
-      locale = document.documentElement.getAttribute('data-localize-lang') || 'en';
-    }
+    const documentLocale = document.documentElement.lang;
+    const localizeLang = document.documentElement.getAttribute('data-localize-lang');
+    const locale = localizeLang || documentLocale || 'en';
     request.headers.set('accept-language', locale);
@@ -26,0 +25,0 @@ }

test/AjaxClient.test.js

@@ -21,2 +21,50 @@ import { expect } from '@open-wc/testing';
 
+  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()', () => {
@@ -134,11 +182,18 @@ it('calls fetch with the given args, returning the result', async () => {
     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);
-      ajax.removeRequestInterceptor(interceptor);
+      const interceptor2 = /** @param {Request} r */ async r =>
+        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`);
     });
@@ -153,4 +208,5 @@
 
-      const response = await (await ajax.request('/foo', { method: 'POST' })).text();
-      expect(response).to.equal('mock response');
+      const response = await ajax.request('/foo', { method: 'POST' });
+      const text = await response.text();
+      expect(text).to.equal('mock response');
     });
@@ -157,0 +213,0 @@ });

test/interceptors-cache.test.js

@@ -1,2 +0,2 @@
-import { expect } from '@open-wc/testing';
+import { aTimeout, expect } from '@open-wc/testing';
 import { spy, stub, useFakeTimers } from 'sinon';
@@ -138,3 +138,3 @@ import '../src/typedef.js';
         removeCacheInterceptors(ajax, indexes);
-      }).to.throw(/Property `requestIdentificationFn` must be of type `function` or `falsy`/);
+      }).to.throw(/Property `requestIdentificationFn` must be of type `function`/);
     });
@@ -456,3 +456,68 @@ });
     });
+
+    it('caches concurrent requests', async () => {
+      newCacheId();
+
+      let i = 0;
+      fetchStub.returns(
+        new Promise(resolve => {
+          i += 1;
+          setTimeout(() => {
+            resolve(new Response(`mock response ${i}`));
+          }, 5);
+        }),
+      );
+
+      const indexes = addCacheInterceptors(ajax, {
+        useCache: true,
+        timeToLive: 100,
+      });
+      const ajaxRequestSpy = spy(ajax, 'request');
+
+      const request1 = ajax.request('/test');
+      const request2 = ajax.request('/test');
+      await aTimeout(1);
+      const request3 = ajax.request('/test');
+      await aTimeout(3);
+      const request4 = ajax.request('/test');
+      const responses = await Promise.all([request1, request2, request3, request4]);
+      expect(fetchStub.callCount).to.equal(1);
+      const responseTexts = await Promise.all(responses.map(r => r.text()));
+      expect(responseTexts).to.eql([
+        'mock response 1',
+        'mock response 1',
+        'mock response 1',
+        'mock response 1',
+      ]);
+
+      ajaxRequestSpy.restore();
+      removeCacheInterceptors(ajax, indexes);
+    });
+
+    it('preserves status and headers when returning cached response', async () => {
+      newCacheId();
+      fetchStub.returns(
+        Promise.resolve(
+          new Response('mock response', { status: 206, headers: { 'x-foo': 'x-bar' } }),
+        ),
+      );
+
+      const indexes = addCacheInterceptors(ajax, {
+        useCache: true,
+        timeToLive: 100,
+      });
+      const ajaxRequestSpy = spy(ajax, 'request');
+
+      const response1 = await ajax.request('/test');
+      const response2 = await ajax.request('/test');
+      expect(fetchStub.callCount).to.equal(1);
+      expect(response1.status).to.equal(206);
+      expect(response1.headers.get('x-foo')).to.equal('x-bar');
+      expect(response2.status).to.equal(206);
+      expect(response2.headers.get('x-foo')).to.equal('x-bar');
+
+      ajaxRequestSpy.restore();
+      removeCacheInterceptors(ajax, indexes);
+    });
   });
 });

No alert changes

Improved metrics

Total package byte prevSize

92517

increased by7.09%

Lines of code

1625

increased by9.87%

No dependency changes