lisaogren / axios-cache-adapter

Caching adapter for axios. Store request results in a configurable store to prevent unneeded network requests.

MIT License

726 stars 107 forks source link

fix(cache): include baseURL in cache key #162

Closed merceyz closed 4 years ago

merceyz commented 4 years ago

What's the problem this PR addresses?

When using axios@0.19.2 the req.url property doesn't include the baseURL. This resulted in requests with the same path but a different baseURL to be cached with the same key.

How did you fix it?

Used whatwg-url to combine req.url and a potential req.baseURL into a full url

codecov[bot] commented 4 years ago

Codecov Report

Merging #162 into master will not change coverage by %. The diff coverage is n/a.

@@            Coverage Diff            @@
##            master      #162   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           12        12           
  Lines          175       177    +2     
=========================================
+ Hits           175       177    +2

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 22f6192...097f62f. Read the comment docs.

merceyz commented 4 years ago

Since it's not important that the key is a valid url a simple solution could be req.baseURL + '-' + req.url, i'm fine either way

merceyz commented 4 years ago

Might be fixed upstream, PR: https://github.com/axios/axios/pull/2751

ghost commented 4 years ago

Awesome job thank you 🙂 I'm ok with the whatwg-url solution, I'll follow what axios decides to do in the end on the PR you mentioned.