feat: simplify file upload #24
Conversation
Flake lock file updates:
• Updated input 'nixpkgs':
'github:NixOS/nixpkgs/f4f322d1424aa547eba9cb092f905f5ceb9b639c' (2024-07-29)
→ 'github:NixOS/nixpkgs/3563397b2f10ffa1891e1a6ce99d13d960d73acd' (2024-07-30)
Flake lock file updates:
• Updated input 'nixpkgs':
'github:NixOS/nixpkgs/3563397b2f10ffa1891e1a6ce99d13d960d73acd' (2024-07-30)
→ 'github:NixOS/nixpkgs/772f92db90637e4560a1d72f86099ccbce359cce' (2024-07-31)
WalkthroughThis update enhances file upload functionality in the project by broadening the types of files that can be uploaded and streamlining the upload process. Additionally, it establishes clearer Node.js version requirements in configuration files. Overall, these changes improve compatibility and robustness in handling file uploads while refining project configurations. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant EntityFiles
participant FormData
participant Blob
User->>EntityFiles: Initiates file upload
EntityFiles->>FormData: Append files
alt File is a string
FormData->>Blob: Convert string to Blob
end
FormData->>EntityFiles: Returns FormData
EntityFiles->>User: Upload successful
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (2)
flake.lockis excluded by!**/*.locksrc/v1/tests/__snapshots__/entity_files_test.ts.snapis excluded by!**/*.snap
Files selected for processing (5)
- flake.nix (1 hunks)
- package.json (1 hunks)
- scripts/build_npm.ts (1 hunks)
- src/v1/entity_files.ts (4 hunks)
- src/v1/tests/entity_files_test.ts (2 hunks)
Files skipped from review due to trivial changes (1)
- flake.nix
Additional comments not posted (9)
package.json (1)
18-20: LGTM! Node.js version requirement added.The addition of the
enginesfield specifying Node.js version>=20ensures that the application runs in a compatible environment.scripts/build_npm.ts (1)
57-59: LGTM! Node.js version requirement added to build configuration.The addition of the
enginesfield specifying Node.js version>=20in the build configuration ensures that the built package declares the required Node.js version.src/v1/tests/entity_files_test.ts (2)
14-16: LGTM! Imports added for path resolution.The imports of
pathandfsmodules and the definition of__dirnamefacilitate path resolution for the new test case.
74-98: LGTM! New test case for uploading files from path added.The new test case for uploading files from a specified path improves the test coverage of the
entityFilesmodule.src/v1/entity_files.ts (5)
68-68: LGTM!The new type
SupportedFileTypecorrectly broadens the file types that can be uploaded.
72-72: LGTM!The updated type
UploadEntityFileRequestcorrectly usesSupportedFileType[].
195-197: LGTM!The
uploadfunction changes streamline the file handling process usingappendToFormData.
226-228: LGTM!The
isFilefunction correctly acts as a type guard forFile.
230-238: LGTM!The
appendToFormDatafunction correctly handles bothFileandstringtypes.
| // TODO(@joscha): replace with `import { openAsBlob } from "node:fs";` ASAP | ||
| function openAsBlob(filePath: string): Promise<Blob> { | ||
| return new Promise((resolve, reject) => { | ||
| const fileStream = fs.createReadStream(filePath) | ||
| const chunks: (string | ArrayBuffer)[] = [] | ||
| fileStream.on('data', (chunk) => { | ||
| chunks.push(typeof chunk === 'string' ? chunk : chunk.buffer) | ||
| }) | ||
| fileStream.on('end', () => { | ||
| resolve(new Blob(chunks)) | ||
| }) | ||
| fileStream.on('error', reject) | ||
| }) | ||
| } |
There was a problem hiding this comment.
LGTM! But replace with standard library function.
The openAsBlob function correctly converts file paths to Blob objects. However, there is a TODO comment indicating it should be replaced with a standard library function.
Do you want me to replace this function with the standard library function or open a GitHub issue to track this task?
Summary by CodeRabbit
New Features
Fileobjects and file paths as strings.Bug Fixes
Tests
Chores
package.jsonto specify a minimum required Node.js version of 20 or higher for better compatibility.