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

Not compatible with axios version 0.27.2, which is in package.json #338

Closed
ckcr4lyf opened this issue Jun 8, 2022 · 4 comments · Fixed by #343
Closed

Not compatible with axios version 0.27.2, which is in package.json #338

ckcr4lyf opened this issue Jun 8, 2022 · 4 comments · Fixed by #343

Comments

@ckcr4lyf
Copy link

ckcr4lyf commented Jun 8, 2022

In this commit support for axios 0.27.2 was introduced via version bump in package.json.

However, axios v0.27 introduces breaking changes which are not properly implemented in https://github.com/ctimmerm/axios-mock-adapter/

Specifically, the error handling is now using a class AxiosError -> axios/axios#3645

However, this library continues to use the old way of generating an axios error ->

function createAxiosError(message, config, response, code) {

As a result, tests fail when some error handling logic for the new axios is tested, as the error from this library is not able to be handled.

This accidental version bump may have come from the assumption that minor version should not have any breaking changes, however as per SemVer in the development stage (0.y.z) anything may change at any time. Ref: https://semver.org/#spec-item-4

Would you accept a PR to fix this?

@ctimmerm
Copy link
Owner

ctimmerm commented Jun 8, 2022

Yes, I'd happily accept a PR for this.

In this commit support for axios 0.27.2 was introduced via version bump in package.json.

That version bump is btw not relevant, that's part of the devDepedencies, which is only used when running tests locally. axios-mock-adapter does not itself have axios as a dependency, it's only listed as a peer depedency, which means that the version of axios that is used is chosen by the project that uses axios-mock-adapter, not by axios-mock-adapter itself.

@ckcr4lyf
Copy link
Author

ckcr4lyf commented Jun 8, 2022

Gotcha. In that case I have a question about the version management:

Wondering how this library can support axios pre 0.27 and 0.27+ at the same time. Or the PR would be merged into v2.x of axios-mock-adapter , and the peerDependency of v1.x would be axios <0.27 and for v2.x it would be >=0.27?

Basically how would someone automatically have the correct version of this library based on their version of axios.

@ctimmerm
Copy link
Owner

ctimmerm commented Jun 8, 2022

We have in the past supported multiple incompatible versions of axios simultaneously. This can be done by doing feature detection, e.g. in this case we know that AxiosError was introduced in v0.27, so if AxiosError is defined, we know that axios >= 0.27 is being used. Then in createAxiosError in axios-mock-adapter, we can use that information and create the same error as before if AxiosError is not defined, and otherwise create an error using the AxiosError constructor.

@ckcr4lyf
Copy link
Author

ckcr4lyf commented Jun 8, 2022

Got it. I will try and work on an MR for this , first without the handling part, then add that in and see if it is sufficient. Thanks for your responses.

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 a pull request may close this issue.

2 participants