-
Notifications
You must be signed in to change notification settings - Fork 451
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(connect): integrations list #2749
Conversation
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.
Feedback to add to the publish.sh script
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.
small comments but lgtm otherwise
packages/connect-ui/src/lib/api.ts
Outdated
return { res, json: json || {} }; | ||
} | ||
|
||
export function isError<TType extends Endpoint<{ Path: any; Success: any; Method: any }>['Reply']>(json: TType): json is Exclude<TType, { data: any }> { |
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.
you used T
above for the generic type name. Let's be consistent
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.
yep you are right, got lazy 🤦🏻
packages/connect-ui/src/lib/api.ts
Outdated
|
||
export async function getIntegrations() { | ||
const { json, res } = await fetchApi<GetPublicListIntegrations>('/integrations', {}); | ||
if (res.status !== 200 || isError(json)) { |
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.
Can this logic be in fetchApi
and make it returns a result. either Ok({resp, json})
or Err(new APIError({res, json})
so we don't need to check in each endpoint.
export async function getIntegrations() {
const res = await fetchApi<GetPublicListIntegrations>('/integrations', {});
if(res.isErr()) {
throw res.error
}
return res.value
}
Thinking about it we can even throw in fetchApi
and only return the json when successful
export async function getIntegrations() {
return fetchApi<GetPublicListIntegrations>('/integrations', {});
}
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.
indeed, not sure why I complicated things
retry: 0, | ||
retryDelay: (attemptIndex) => { | ||
return Math.min(2000 * 2 ** attemptIndex, 30000); | ||
} |
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.
do you need do define retryDelay if retry = 0?
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.
no but I might reenable retry at some point, it was annoying now because the backend is mostly broken :p
## Describe your changes Contributes to https://linear.app/nango/issue/NAN-1703/create-ui - Create the integrations list page [[figma](https://www.figma.com/design/wBPwGMlopg8tcMFRekYLgE/Auth-UI?node-id=4107-456&node-type=canvas&m=dev)] - Setup react correctly: error boundary, router, fetcher, <img width="488" alt="Screenshot 2024-09-20 at 13 54 26" src="https://github.com/user-attachments/assets/9954f917-c3b4-4846-9849-8506341abd45"> ## 🧪 Tests ``` npm run dw npm run dwa # temp to be able to use API touch packages/connect-ui/.env VITE_LOCAL_SECRET_KEY=__SECRET_KEY__ npm run connect-ui:dev:watch ```
Describe your changes
Contributes to https://linear.app/nango/issue/NAN-1703/create-ui
Create the integrations list page [figma]
Setup react correctly: error boundary, router, fetcher,
🧪 Tests