feat(storage): add version parameter for download#2153
feat(storage): add version parameter for download#2153Fruup wants to merge 5 commits intosupabase:masterfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
|
Hi @Fruup, thanks for the PR! Worth noting that we recently merged #2090 which addresses the same underlying stale-cache problem, but at the HTTP fetch level ( Before I can merge this, may I ask for a few things please:
Happy to review once those are addressed! Thank you so much for contributing to Supabase! 💚 |
mandarini
left a comment
There was a problem hiding this comment.
Read my comment please
|
@mandarini Thank you for your message. I implemented your feedback :) |
@supabase/auth-js
@supabase/functions-js
@supabase/postgrest-js
@supabase/realtime-js
@supabase/storage-js
@supabase/supabase-js
commit: |
mandarini
left a comment
There was a problem hiding this comment.
Great work addressing the previous feedback!!! Thank you so much! :D A few more things before this is ready to merge:
1. Trailing & in createSignedUrl
When no download or version is passed, query.toString() returns "", producing a URL like ...?token=abc&. Fix:
const queryString = query.toString()
const signedUrl = encodeURI(`${this.url}${returnedPath}${queryString ? `&${queryString}` : ''}`)2. Same issue in createSignedUrls
? encodeURI(`${this.url}${datum.signedURL}${queryString ? `&${queryString}` : ''}`)3. Missing test for download with version
The new test covers createSignedUrl, getPublicUrl, and createSignedUrls but not download. Since the test suite runs against real infrastructure, please add a proper integration test alongside the others:
// `download` with version
{
const res = await storage.from(bucketName).download(uploadPath, { version })
expect(res.error).toBeNull()
assert(res.data)
}Thank you so much for your patience!! :D
| paths: string[], | ||
| expiresIn: number, | ||
| options?: { download: string | boolean } | ||
| options?: { download: string | boolean; version?: string } |
There was a problem hiding this comment.
typescript won't be happy, setting version requires download now so it means there is test gap
🔍 Description
As the documentation already suggests (see here), I implemented a version parameter that can be attached to the download request.
What changed?
Why was this change needed?
Currently, the Supabase CDN unavoidably caches files for 1 minute. This leads to stale data being served after a file is replaced.
📸 Screenshots/Examples
🔄 Breaking changes
📋 Checklist
<type>(<scope>): <description>npx nx formatto ensure consistent code formatting📝 Additional notes