-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
swap axios for fetch in the JS client #409
Conversation
@jeffchuber for me this does not matter. Feel free to merge this and close my PR :) |
@gustawdaniel roger that. all the credit to you - thanks for figuring out the hard edges here! |
// we need to override constructors to make it work with jest | ||
// https://stackoverflow.com/questions/76007003/jest-tobeinstanceof-expected-constructor-array-received-constructor-array | ||
function repack(value: unknown): any { | ||
if (Boolean(value) && typeof value === "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.
does if value give the same semantics here?
function repack(value: unknown): any { | ||
if (Boolean(value) && typeof value === "object") { | ||
if (Array.isArray(value)) { | ||
return new Array(...value); |
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 we need to handle nested values?
clients/js/src/types.ts
Outdated
Documents = 'documents', | ||
Embeddings = 'embeddings', | ||
Metadatas = 'metadatas', | ||
Distances = 'distances' |
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 unused?
Approved pending comments! |
Can you merge this one? |
All the good stuff here is credit of @gustawdaniel . This separate PR is mainly handling merge conflicts from
main
moving on with additional functionality.#348
This PR
axios
forfetch
per JS/TS + Misc Feedback #254prettier
@gustawdaniel feel free to push up this PR separately and I'll merge yours instead
TODO: bump
npm
and cut new release