Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ajax): add interceptors for parsed response JSON objects #2335

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bashmish
Copy link
Contributor

What I did

  1. added addResponseJsonInterceptor to be able to intercept parsed response JSON objects
  2. added docs and tests for it

Copy link

changeset-bot bot commented Aug 20, 2024

🦋 Changeset detected

Latest commit: 3ed0929

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@lion/ajax Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@CLAassistant
Copy link

CLAassistant commented Aug 20, 2024

CLA assistant check
All committers have signed the CLA.

@bashmish bashmish force-pushed the feat/json-interceptor branch 2 times, most recently from 12e129d to 3e8c21f Compare August 20, 2024 17:01
@tlouisse
Copy link
Member

@ing-athirlwall could you maybe have a look?

@ing-athirlwall
Copy link
Contributor

LGTM. Perhaps use "forEach" instead of "for..of" when iterating through the interceptors but that's just a niggle.

@bashmish
Copy link
Contributor Author

bashmish commented Sep 24, 2024

LGTM. Perhaps use "forEach" instead of "for..of" when iterating through the interceptors but that's just a niggle.

as discussed in the chat, it was implemented the same way as other interceptor handlers, so should be good for code consistency :) e.g. this one:

  /**
   * @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(/** @type {CacheResponse} */ (interceptedResponse));
    }
    return interceptedResponse;
  }

@ing-athirlwall
Copy link
Contributor

Fair enough .. like the feature, let's release it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants