refactor(storage)!: make Storage client types stateless structs - #1246
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 1 remains after this review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughStorage APIs now use immutable Sequence Diagram(s)sequenceDiagram
participant SupabaseClient
participant SupabaseStorageClient
participant StorageFileApi
participant StorageApi
SupabaseClient->>SupabaseStorageClient: storage
SupabaseStorageClient->>StorageFileApi: from(bucket)
StorageFileApi->>StorageApi: execute(file request)
StorageApi-->>StorageFileApi: response
StorageFileApi-->>SupabaseStorageClient: decoded result
Possibly related PRs
Suggested labels: Merge Risk: ⚪ Minimal · up to This change refactors storage client state handling and documents the breaking API updates; no actionable merge-blocking risk remains beyond normal checks and review. ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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 |
StorageApi, StorageBucketApi, StorageFileApi, and SupabaseStorageClient convert from a LockIsolated-backed class hierarchy to immutable structs composing a StorageApi value, mirroring the prior FunctionsClient conversion (#1233). setHeader on StorageApi and StorageFileApi is now non-mutating and returns a new instance instead of mutating in place. Bucket-management operations move from the deleted StorageBucketApi class into a SupabaseStorageClient extension in SupabaseStorageClient+Buckets.swift.
…lient/StorageFileApi The stateless-struct refactor made SupabaseStorageClient.configuration and StorageFileApi.configuration unreachable without @testable import, an extra undocumented breaking change beyond the refactor's scope since configuration was public on the original class hierarchy. Add forwarding computed properties on both types and drop the now-unnecessary .api. test workarounds.
The composition-based refactor dropped setHeader(_:forKey:) from SupabaseStorageClient, which previously inherited it transitively through StorageBucketApi -> StorageApi. Restore it as a public, non-mutating method forwarding to the held StorageApi, matching the same pattern already used for the restored configuration property.
spydon
approved these changes
Aug 20, 2026
Add a concrete "Why" to the V3_MIGRATION.md struct-conversion section: the old SupabaseStorageClient.from(_:) built each StorageFileApi from the immutable configuration alone, never the live header state setHeader(_:forKey:) mutated, so a header set via setHeader silently never reached file operations obtained via from(_:) (while it did reach storage.vectors, which passed self through instead). The composition-based rewrite passes the whole api value through both paths, fixing the asymmetry. Also soften the "StorageBucketApi was never constructed directly" claim to scope it to this codebase (it was a public class with an inherited public initializer, so external code could have constructed one), and document that SupabaseClient.storage stopped memoizing its result, so a header set via client.storage.setHeader(...) no longer persists across later client.storage accesses.
DocC doesn't render doc comments on extension declarations, so the "## Topics" block on the SupabaseStorageClient bucket-management extension rendered nowhere. The same 6 symbol links already appear under "Bucket management" in SupabaseStorageClient's own type-level doc comment, so trim this one down to a plain descriptive comment.
SupabaseClient.storage stopped caching its result in mutableState, so discarding client.storage immediately can no longer exercise the same retain-cycle detection as the cached sub-clients (rest, functions, realtimeV2): nothing keeps a self-capturing closure it might build alive past the statement, cached or not. Move it out of the "cached sub-clients" list and explain why it can't prove the same thing.
SupabaseStorageClient.configuration has direct test coverage; its sibling StorageFileApi.configuration did not, despite being the exact class of member that was accidentally dropped and restored once already during the stateless-struct refactor.
StorageApi held no public API surface anyone outside Sources/Storage could actually use: execute(_:) was already internal, and neither SupabaseStorageClient nor StorageFileApi exposed a way to build one from a standalone StorageApi value. Narrow it while v3 is still unreleased, since this is the cheapest moment to do so.
grdsdev
enabled auto-merge (squash)
August 20, 2026 14:40
grdsdev
force-pushed
the
claude/sources-storage-stateless-5b0058
branch
from
August 20, 2026 14:43
19bac7c to
1795f4f
Compare
Coverage Report for CI Build 32381856545Coverage increased (+0.04%) to 87.138%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Converts
Sources/Storage's public client types (StorageApi,StorageBucketApi,StorageFileApi,SupabaseStorageClient) fromLockIsolated-backed classes to immutable structs, mirroring theFunctionsClientconversion in #1233. Part of the "supabase-swift v3" migration (tracked in SDK-1533).StorageApibecomes a plain immutable struct (dropsLockIsolated/@unchecked Sendable), and is nowinternal— nothing public ever accepted or returned one, so it was dead public surface (SDK-1534).StorageBucketApiis removed; its methods move ontoSupabaseStorageClientdirectly (SupabaseStorageClient+Buckets.swift).SupabaseStorageClientandStorageFileApibecome structs composingStorageApi, matching the shape already used by the Vectors trio (StorageVectorsClient/VectorBucketClient/VectorIndexClient, untouched).setHeader(_:forKey:)is now non-mutating (returns a new value) and drops@discardableResult.SupabaseClient.storagestops being memoized, mirroring the existingfunctionsproperty.SupabaseStorageClient.from(_:)used to silently drop headers set viasetHeaderon the top-level client (it only passed the storedconfiguration, not the live header state) — the composition-based rewrite fixes this asymmetry.V3_MIGRATION.mddocuments all of the above as breaking changes.setHeader's name should change now that it's non-mutating).Test plan
swift test— 1182/1182 passing, 9 known pre-existing issues, no new failures./scripts/test-docs.sh— clean, no DocC warnings./scripts/spell-check.sh— clean on all shipped files./scripts/format.sh— no changesSupabase.xcworkspace,Examplesscheme) — builds successfully