fix(storage): improve FileObject type accuracy with nullable fields#2116
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds a FileMetadata interface and applies it across storage types, replacing prior generic metadata. FileObject fields id, updated_at, created_at, last_accessed_at, and metadata are now nullable and legacy fields (bucket_id, owner, buckets) are optional/deprecated. A new FileObjectV2 models detailed info responses (id, version, name, bucket_id, created_at, last_modified preferred over updated_at, optional size/cache_control/content_type/etag/metadata). SearchV2Object and SearchV2Folder were added/reshaped to include name, id, timestamps and FileMetadata. SearchV2Result now includes folders, objects, hasNext and optional nextCursor. StorageFileApi JSDoc and tests were updated to reflect richer response shapes. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@supabase/auth-js
@supabase/functions-js
@supabase/postgrest-js
@supabase/realtime-js
@supabase/storage-js
@supabase/supabase-js
commit: |
130799e to
f747aa5
Compare
475e161 to
6cee1be
Compare
6cee1be to
4fa1627
Compare
4fa1627 to
8b8d435
Compare
|
@itslenny addressed your comment |
4e23635 to
e33804e
Compare
e33804e to
d44a4c0
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/core/storage-js/src/lib/types.ts (1)
132-133:⚠️ Potential issue | 🟠 Major
FileObjectV2.metadatalooks over-constrained forinfo()responses (Line 133).
FileObjectV2already models system metadata via top-level fields (size,cache_control,content_type,etag,last_modified). TypingmetadataasFileMetadataforces required system keys again and can mis-type validinfo()payloads when this field is arbitrary/user metadata.Proposed type fix
export interface FileObjectV2 { @@ - /** Custom file metadata */ - metadata?: FileMetadata + /** Custom/user metadata returned by info endpoint */ + metadata?: Record<string, any> | nullAs per coding guidelines, "Comment only when the issue must be resolved before merge" and "focus strictly on merge-blocking concerns."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/storage-js/src/lib/types.ts` around lines 132 - 133, The FileObjectV2.metadata field is too strict (it re-enforces system keys from FileMetadata and can mis-type arbitrary user metadata returned by info()); change the metadata property on FileObjectV2 from FileMetadata to a loose map type (e.g., metadata?: Record<string, unknown> or Record<string, any>) so it accepts arbitrary/user metadata, and update any consumers of FileObjectV2.metadata to handle the relaxed shape safely (e.g., runtime checks or key-safe access).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/core/storage-js/src/lib/types.ts`:
- Around line 132-133: The FileObjectV2.metadata field is too strict (it
re-enforces system keys from FileMetadata and can mis-type arbitrary user
metadata returned by info()); change the metadata property on FileObjectV2 from
FileMetadata to a loose map type (e.g., metadata?: Record<string, unknown> or
Record<string, any>) so it accepts arbitrary/user metadata, and update any
consumers of FileObjectV2.metadata to handle the relaxed shape safely (e.g.,
runtime checks or key-safe access).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 4449fffa-584b-4b57-a880-5608379ce198
📒 Files selected for processing (2)
packages/core/storage-js/src/lib/types.tspackages/core/storage-js/src/packages/StorageFileApi.ts
4 participants
A clarification on the changes, and whether they are breaking or not:
At runtime, nothing changes. The Storage API has always returned null for id, updated_at, created_at, last_accessed_at, and metadata on folder entries. This is explicitly declared as nullable in the storage/schemas/object.ts. Likewise, bucket_id, owner, and buckets have never been included in list() responses. The previous TypeScript types were incorrect: they promised non-null values that the server never guaranteed, meaning any user code that relied on those guarantees was already silently broken at runtime for folder entries. This PR corrects that lie, the types now accurately reflect what the server has always actually returned. No JavaScript or TypeScript behavior changes. Only the type checker will now correctly surface cases where null-safety wasn't being handled.