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 Axios with regular fetch #117

Merged
merged 2 commits into from
Dec 5, 2023
Merged

Replace Axios with regular fetch #117

merged 2 commits into from
Dec 5, 2023

Conversation

Skaldebane
Copy link
Contributor

In environments like Cloudflare Workers and Pages Functions, Axios requests don't work because XMLHttpRequests are not supported.

Replacing it with a regular fetch fixes this.

@CLAassistant
Copy link

CLAassistant commented Dec 4, 2023

CLA assistant check
All committers have signed the CLA.

@lukasIO
Copy link
Contributor

lukasIO commented Dec 4, 2023

Not against this change, but we have to keep in mind that fetch is only available in node without flags since v18.
For a v2, I guess we could also bump the minimum node requirement to v18. But we should make sure that other environments that we want to target, also support fetch ootb.

@davidzhao
Copy link
Member

should we use something like node-fetch for node env?

@lukasIO
Copy link
Contributor

lukasIO commented Dec 5, 2023

IIRC there were some inconsistencies between native fetch and node-fetch last time I came across it.
Ideally we wouldn't need to include it.
I did a quick check for other popular JS runtimes and they all implement fetch ootb, so I guess we're fine as long as we bump nodeJS requirement to v18, which makes sense as everything else has reached EOL already.

@davidzhao
Copy link
Member

Thanks @lukasIO, merging this.

@davidzhao davidzhao merged commit 9dd604f into livekit:main Dec 5, 2023
1 check passed
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