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

[TECH] Upgrade Axios to latest #3917

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[TECH] Upgrade Axios to latest #3917

wants to merge 2 commits into from

Conversation

flavioislima
Copy link
Member

Upgrade Axios to latest version and also fix the necessary types and imports


Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

@flavioislima flavioislima self-assigned this Aug 9, 2024
@flavioislima flavioislima added the pr:ready-for-review Feature-complete, ready for the grind! :P label Aug 9, 2024
Copy link
Collaborator

@CommandMC CommandMC left a comment

Choose a reason for hiding this comment

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

Other than the one type issue, LGTM. Might want to wait until after the release is done to merge this though

@@ -869,7 +871,7 @@ export async function getBuilds(
url.searchParams.set('password', password)
}

const headers: AxiosRequestHeaders = {}
const headers = {} as AxiosRequestHeaders
Copy link
Collaborator

Choose a reason for hiding this comment

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

This type assertion is incorrect. Most of the time, if you add as T because TS is yelling at you, you're doing something wrong. In this case, {} can't be of type AxiosHeaders (AxiosHeaders is a class with a few methods, which an empty object of course doesn't have)

Axios also accepts a standard Record for headers, which is fine to use here:

Suggested change
const headers = {} as AxiosRequestHeaders
const headers: Record<string, string> = {}

@@ -1265,7 +1267,7 @@ export async function getProductApi(
url.searchParams.set('expand', expand.join(','))
}

const headers: AxiosRequestHeaders = {}
const headers = {} as AxiosRequestHeaders
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same issue as above here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:ready-for-review Feature-complete, ready for the grind! :P
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants