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

feat(js client): improve things (one axios client, keepAlive, baseUrl… #47

Merged
merged 3 commits into from
Sep 4, 2024

Conversation

GuillaumeDecMeetsMore
Copy link
Collaborator

@GuillaumeDecMeetsMore GuillaumeDecMeetsMore commented Sep 3, 2024

Changed

  • Refactor client lib so that
    • baseUrl can be provided in the parameter of the function creating a new Client
    • Use it in the Axios instance's configuration so that it doesn't need to be passed later
    • Make it so that only one Axios instance is created per client, instead of one per "subclass"
      • This means in most cases, only 1
    • For the admin client, allow to have keepAlive connections

@GuillaumeDecMeetsMore GuillaumeDecMeetsMore self-assigned this Sep 3, 2024
@GuillaumeDecMeetsMore GuillaumeDecMeetsMore marked this pull request as ready for review September 3, 2024 06:42
constructor(nettuAccount: string, token?: string) {
this.nettuAccount = nettuAccount
this.token = token
export const createAxiosInstanceFrontend = (

Choose a reason for hiding this comment

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

Can you document? What's the difference between this and createAxiosInstanceBackend?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated 👍

The main difference is that one allow keepAlive, which needs the function to be async as it needs to import a Node package.

Technically, it could be only one function, but I think it makes it easier for the frontend to still have a function that isn't async. That way, it can be defined directly as a variable in a file (export ....) on the frontend

/**
* Nettu account id (admin)
*/
nettuAccount?: string

Choose a reason for hiding this comment

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

Nettu or Nettei?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For the moment nettu is still used everywhere
I think I'll just make one big PR renaming everything to nittei

@GuillaumeDecMeetsMore GuillaumeDecMeetsMore merged commit 3d2d21e into master Sep 4, 2024
2 checks passed
@GuillaumeDecMeetsMore GuillaumeDecMeetsMore deleted the guillaume/feat/improve-js-client branch September 4, 2024 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants