-
Notifications
You must be signed in to change notification settings - Fork 181
feat: node-appwrite use ts #799
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
Conversation
} | ||
return 'string[]'; | ||
case self::TYPE_FILE: | ||
return "File"; |
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.
This should still be InputFile
right? Otherwise the storage service param will expect a File
?
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've switched this to File
on purpose. The InputFile class now returns an instance of the File class.
File is a web Javascript standard API[1] and is available on most edge runtimes including vercel[2].
It's available in Node.js from version 18, and we use node-fetch-native
to fallback on a polyfill where it doesn't exist.
[1] https://developer.mozilla.org/en-US/docs/Web/API/File
[2] https://nextjs.org/docs/pages/api-reference/edge
[3] https://nodejs.org/docs/latest/api/buffer.html#class-file
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 this change the service method parameter types? We can use File
internally, but should still expect an InputFile
in the user-facing methods like storage.createFile
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.
Yes, because the user's InputFile
helpers import will break on edge runtimes (due to its dependency on Node's fs
module), so if we want storage.createFile
to work on edge runtimes, we'll have to use the builtin File
object.
If we use InputFile
in the service method parameters then it is more obvious to the user that the helpers exist, but it will only work in Node-like environments
@loks0n, feel free to re-request a review once you're ready for me to review again. |
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.
Let's get this released as an RC release and see how it performs.
Been using it for over a week now and it's pretty solid. |
node-appwrite
SDK to TypeScript.InputFile
module that depends onfs
.I've published a test package at
luke-node-appwrite-edge@0.0.32
Valuable context/reading:
Closes #652 appwrite/sdk-for-node#47