-
Notifications
You must be signed in to change notification settings - Fork 349
feat: base api-client impl #4694
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
base: main
Are you sure you want to change the base?
Conversation
|
We will figure out publishing at a later date. |
Prospector
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall, quite cool but quite concerned that the tauri implementation is not doing what we want
| * Nuxt-specific configuration | ||
| */ | ||
| export interface NuxtClientConfig extends ClientConfig { | ||
| // TODO: do we want to provide this for tauri+base as well? its not used on app |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as a config, sure I don't see why not, since it is something the API supports. if we ever build another server-side JS-based application, it could be used. but yeah, can't use a ratelimit key for the app since it'd be client-side
| @@ -0,0 +1,115 @@ | |||
| export type Environment = 'required' | 'optional' | 'unsupported' | 'unknown' | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if a better way to organize these types would be hierarchical? I noticed it's weird how we have like ProjectV2, etc. for some things but other things are versionless.
Perhaps the export here should be like a V2Types block, with types within it? So to get a V2 project type you'd use LabrinthV2.Project ? This way I think you could also have some common types that are shared in another file and exported in both LabrinthV2 and LabrinthV3 if they are the same in both
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decided to use the structure: Labrinth.V2.Project
| protected async executeRequest<T>(url: string, options: RequestOptions): Promise<T> { | ||
| try { | ||
| // @ts-expect-error - $fetch is provided by Nuxt runtime | ||
| const response = await $fetch<T>(url, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe $fetch should be passed in to the client construction so we don't need to suppress this error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will install nuxt types instead, or do what we do for the tauri platform, dynamic import.
| } | ||
|
|
||
| // @ts-expect-error - import.meta is provided by Nuxt | ||
| if (import.meta.server && this.config.rateLimitKey) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here, couldn't we pass whether it's client or server at construction/in config?
| } | ||
|
|
||
| /** | ||
| * Tauri platform client using Tauri v2 HTTP plugin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
im confused how this works. is this not just bypassing the whole tauri backend that does the caching and processing on the backend?
the way I envisioned this would work is we'd implement like a partial API client using the invoke functions when they exist, and if not then we fallback to making a simple web request (using user's auth and stuff). but web requests should be a fallback if it's not provided by the app backend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mentioned on slack;
- can enable caching via plugin config
- we can still have invoke stuff for the routes that need it (i'll design a way for this to happen when we need to do it)
Uh oh!
There was an error while loading. Please reload this page.