lisaogren/axios-cache-adapter

2.6.0 Adding ability to cache requests made with methods beyond GET is a breaking change

jcass8695 opened this issue · 5 comments

jcass8695 commented

Hello 👋

Today we had the below unit test fail, due to the change introduced in 2.6.0 that now automatically caches requests made with HTTP verbs outside of GET (i.e. POST, PUT etc.). I think the behaviour should be for consumers to opt-in to this enhancement through a config option, and the default behaviour should remain. Otherwise it is a breaking change and should result in a major version bump.

it('should not cache the request', async () => {
      nock(baseUrl, { reqheaders }).post(path, body).twice().reply(200, validResponse);
      const firstResponse = await service.post(`${baseUrl}${path}`, body, reqheaders, operation);
      const secondResponse = await service.post(`${baseUrl}${path}`, body, reqheaders, operation);
      expect(firstResponse.data).to.deep.equal(validResponse);
      expect(secondResponse.data).to.deep.equal(validResponse);
      expect(firstResponse.request.fromCache).to.be.undefined;
      expect(secondResponse.request.fromCache).to.be.undefined; // Test fails on this line with true != undefined
    });
lainas262 commented

I agree with the above. Codebases using the adapter make the assumption that only GET requests get cached and now need to go through a migration process to update all calls with other methods to control caching. I may be wrong, just sharing my thought 😄

Deleted user commented

Hello @JCass45 really sorry for introducing this breaking change. Will work on restoring previous default behavior of caching only GET requests and introduce a specific option to extend it to other methods.

jcass8695 commented

@RasCarlito 🙏🏻 Appreciate it!

jcass8695 commented

Yep confirmed 👍