-
Notifications
You must be signed in to change notification settings - Fork 115
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
Allow using shared extractors #54
Conversation
@ccurme creating a separate endpoint for the shared extractor. I am not 100% sure this is the way to go, but was thinking about trying to keep the semantics clear about when we're using something shared vs. not |
frontend/app/utils/api.tsx
Outdated
type ExtractionResponse = { | ||
data: 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.
Is this data structure actually unknown? If possible we should type it
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.
Also build failing because unknown
is not assignable to Record<string, any>[]
.
Three options:
- switch type to
Record<string, any>[]
if we know the response will always be a dict, then this is valid - switch to
any[]
- add typing for the response object
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 I'll adjust in a bit
@@ -21,6 +21,9 @@ type GetExtractorQueryKey = [string, string, boolean]; // [queryKey, uuid, isSha | |||
|
|||
type OnSuccessFn = (data: { uuid: string }) => void; | |||
|
|||
axios.defaults.withCredentials = true; |
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.
@bracesproul I will need your help to figure out how to do this correctly later on. For now going to merge to get unblocked
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.
The other way to do it would be via the config object when actually performing the axios request:
const response = await axios.get(`${baseUrl}/shared/extractors/${uuid}`, {
withCredentials: true,
});
Allow using shared extractors.
This won't work with current version of backend due to cookies not being passed