Skip to content

fix(storage): omit undefined limit and offset from listBuckets query - #2781

Open
prestonschlagheck wants to merge 1 commit into
supabase:masterfrom
prestonschlagheck:fix/storage-list-buckets-undefined-pagination
Open

prestonschlagheck wants to merge 1 commit into
supabase:masterfrom
prestonschlagheck:fix/storage-list-buckets-undefined-pagination

Conversation

@prestonschlagheck

@prestonschlagheck prestonschlagheck commented Oct 21, 2026 •

Copy link
Copy Markdown

🔍 Description

What changed?

StorageBucketApi.listBuckets now only adds limit and offset to the query string when they are defined. Before, it checked 'limit' in options / 'offset' in options.

Why was this change needed?

A key can be present but undefined. That happens with a common call like storage.listBuckets({ limit: pageSize, offset }) when the caller has no page size set. In that case the request was sent as:

GET /storage/v1/bucket?limit=undefined&offset=undefined

StorageAnalyticsClient.listBuckets already uses !== undefined for the same options, so this makes the two consistent. 0 is still sent.

📸 Screenshots/Examples

await storage.listBuckets({ limit: undefined, offset: undefined, search: 'avatars' })
// before: /bucket?limit=undefined&offset=undefined&search=avatars
// after:  /bucket?search=avatars

🔄 Breaking changes

  • This PR contains no breaking changes

📋 Checklist

  • I have read the Contributing Guidelines
  • My PR title follows the conventional commit format: <type>(<scope>): <description>
  • I have run pnpm nx format to ensure consistent code formatting
  • I have added tests for new functionality (if applicable)
  • I have updated documentation (if applicable) — n/a

📝 Additional notes

Added two unit tests in storageBucketApi.test.ts:

  • Explicitly undefined limit/offset are omitted. This test fails on master.
  • limit: 0, offset: 0 are still sent.

The storage-js package compiles its tests with exactOptionalPropertyTypes, so the first test casts its options to model a JS caller, or a TS caller without that flag.

@prestonschlagheck
prestonschlagheck requested review from a team as code owners October 21, 2026 18:45
@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c76cbcc1-356c-4650-9329-3062e4c0fe29
📥 Commits

Reviewing files that changed from the base of the PR and between f5afba4 and 5909d81.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 07b49126-82e5-4756-848e-6fed6079d6eb
📥 Commits

Reviewing files that changed from the base of the PR and between 5adfffe and f5afba4.

📒 Files selected for processing: 2
  • packages/core/storage-js/src/packages/StorageBucketApi.ts
  • packages/core/storage-js/test/storageBucketApi.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Corrected bucket listing requests so explicitly undefined pagination values are omitted from the query.
    • Preserved valid zero values for pagination limits and offsets.
    • Continued to include search criteria when provided.

The listBuckets query builder now omits limit and offset when their values are undefined. Tests verify that search remains in the request and that zero-valued pagination options are included.

Priority: ⬇️ Low

Change: Bug fix

Merge Risk: ⚪ Minimal · up to f5afb

The pagination query handling is mergeable with no identified user-facing or operational risk.

- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

listBuckets checked `'limit' in options` / `'offset' in options`, so a key
that was present but undefined (e.g. `listBuckets({ limit: pageSize })` with
no page size set) was sent as the literal string `limit=undefined`. Check for
`!== undefined` instead, matching StorageAnalyticsClient.listBuckets; `0` is
still sent.
@prestonschlagheck
prestonschlagheck force-pushed the fix/storage-list-buckets-undefined-pagination branch from f5afba4 to 5909d81 Compare October 21, 2026 18:46

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant