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

Replace fetch by axios #279

Merged
merged 3 commits into from
Feb 2, 2022

Conversation

fschmenger
Copy link
Collaborator

This is a proposed cleanup task by @JakobMiksch from #269. The Fetch API is replaced by axios, no functional changes.

@fschmenger
Copy link
Collaborator Author

Please re-run the tests after #269 has been merged, as I didn't include the axios package here again to avoid conflicts (already included in #269).

Open task:
I didn't dare to touch the geocoder requests, those already have been marked with.
https://github.com/meggsimum/wegue/blob/abb2b3518d54b0390ffefe765e0c568122631448/src/components/geocoder/helpers/ajax.js#L24

As far as I can see, the involved JSONP requests require a separate JsonP adapter. However from looking at the providers, none is currently configured to use JSONP, so the corresponding code might as well be deprecated.
Since I'm not familiar with all the geocoder provider specs and I didn't have the time to go through dozens of tests, I'll leave this for later.

Copy link
Collaborator

@JakobMiksch JakobMiksch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix. I will merge.

@JakobMiksch JakobMiksch merged commit f21496b into wegue-oss:master Feb 2, 2022
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.

2 participants