refactor(storage): move toQueryParams onto TransformOptions - #1672
Conversation
The ToQueryParams extension added a getter to a class in the same library, so it can be a plain member instead. Marked @internal to keep it out of the public API, matching DownloadBehavior.queryValue.
github-actions
Bot
added
the
storage
📝 WalkthroughWalkthroughThe change documents ChangesToQueryParams visibility
Estimated code review effort: 1 (Trivial) | ~2 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/storage_client/lib/storage_client.dart`:
- Line 11: Update the storage_client package barrel export in
storage_client.dart so the internal TransformOptions.toQueryParams API is not
exposed publicly. Remove the broad src/types.dart export or use an export
restriction that hides toQueryParams while preserving the intended public types.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fe910085-e1ce-4ef9-80e2-a5e771a23fe2
📒 Files selected for processing (2)
packages/storage_client/lib/src/types.dartpackages/storage_client/lib/storage_client.dart
| export 'src/storage_client.dart'; | ||
| export 'src/storage_file_api.dart'; | ||
| export 'src/types.dart' hide ToQueryParams; | ||
| export 'src/types.dart'; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Repository files matching storage_client.dart/types.dart:\n'
git ls-files | rg '(^packages/storage_client/lib/(storage_client|src/types)\.dart$|pubspec\.yaml$)'
printf '\nnearby storage_client.dart:\n'
cat -n packages/storage_client/lib/storage_client.dart | sed -n '1,40p'
printf '\nsrc/types.dart outline/header and relevant symbols:\n'
cat -n packages/storage_client/lib/src/types.dart | sed -n '1,220p'
printf '\nSearch for TransformOptions and toQueryParams usage/exports:\n'
rg -n "TransformOptions|toQueryParams|export .*types\.dart|`@internal`|pubspec" packages/storage_client -SRepository: supabase/supabase-flutter
Length of output: 13125
🌐 Web query:
Dart meta package deprecated @internal exports library public API
💡 Result:
The @internal annotation in the Dart meta package is not deprecated; it remains the standard, supported way to mark declarations intended for use only within the package that defines them [1][2]. Contrary to the suggestion that @internal exports a public API, the annotation is specifically designed to prevent elements from being exposed as part of a package's public API [1][3]. The Dart analyzer enforces this by triggering diagnostics if you attempt to use it incorrectly [4][5]: 1. invalid_internal_annotation: This error occurs if you apply @internal to a declaration in a public library (a library outside of lib/src) or to an element with a private name [4]. It is intended only for private APIs within your package [6]. 2. invalid_export_of_internal_element: This error occurs if you try to export a declaration marked with @internal from a public library [5][7]. If you have code marked with @internal that is currently appearing in your public API, the analyzer expects you to either remove the @internal annotation (if it is meant to be public) or move the annotated declaration into a library inside the lib/src directory and hide it from your public exports using a hide clause [4][5]. It is not a mechanism for "internal exports" but a mechanism to guard against the accidental exposure of internal implementation details [1][8].
Citations:
- 1: https://pub.dev/documentation/meta/latest/meta/internal-constant.html
- 2: https://pub.dev/documentation/meta/latest/meta/
- 3: https://github.com/dart-lang/sdk/blob/master/pkg/meta/lib/meta.dart
- 4: https://dart.dev/tools/diagnostics/invalid_internal_annotation
- 5: https://dart.dev/tools/diagnostics/invalid_export_of_internal_element
- 6: missing
invalid_internal_annotationHint on @internal annotation on public constructor in public API dart-lang/sdk#47744 - 7: https://deepsource.com/directory/dart-analyze/issues/DRT-W1414
- 8: Add a hint: "@internal" annotation dart-lang/sdk#28066
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
barrel = Path('packages/storage_client/lib/storage_client.dart')
src_types = Path('packages/storage_client/lib/src/types.dart')
to_query = Path('packages/storage_client/lib/src/types.dart').read_text()
# Parse only relevant directives and annotation context from the two files.
barrel_exports = [line.strip() for line in barrel.read_text().splitlines() if line.strip().startswith("export '")]
src_exports = [line.strip() for line in src_types.read_text().splitlines() if line.strip().startswith("export '")]
# Extract TransformOptions declaration and the toQueryParams block.
lines = to_query.splitlines()
start = None
for i, line in enumerate(lines, 1):
if line.startswith('class TransformOptions'):
start = i
break
if start:
block = '\\n'.join(lines[start-1:])
block_lines = block.split('\\n')
for j, bl in enumerate(block_lines):
if 'Map<String, String> get toQueryParams {' in bl:
context = block_lines[max(0, j-3):j+5]
print('TransformOptions.get toQueryParams:')
for idx_offset, cl in enumerate(context, max(0, j-3)):
print(f'{start+j-1+idx_offset}: {cl}')
print('contains_internal_annotation_on_block=' + str('`@internal`' in '\\n'.join(context)))
print('barrel_exports_contain_src/types.dart=' + str("export 'src/types.dart';" in barrel_exports))
print('export_uses_hide_clause_for_toQueryParams=' + str('toQueryParams' not in barrel.read_text()))
PYRepository: supabase/supabase-flutter
Length of output: 633
Do not export TransformOptions.toQueryParams from the package barrel.
packages/storage_client/lib/storage_client.dart exports the whole src/types.dart library, so TransformOptions.toQueryParams is reachable through the public API despite its @internal annotation. Remove this export or hide toQueryParams from the export if the getter must remain internal.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/storage_client/lib/storage_client.dart` at line 11, Update the
storage_client package barrel export in storage_client.dart so the internal
TransformOptions.toQueryParams API is not exposed publicly. Remove the broad
src/types.dart export or use an export restriction that hides toQueryParams
while preserving the intended public types.
1 participant
ToQueryParamswas an extension onTransformOptionsdeclared in the same library as the class it extended, so it did not need to be an extension at all. It is now a plain getter on the class.To keep it out of the public API (it was previously hidden via
export 'src/types.dart' hide ToQueryParams;) the getter is marked@internal, which matches howDownloadBehavior.queryValueis handled in the same file. Since@internalsymbols are skipped by the compliance matrix symbol check, no matrix update is needed.I also checked the three other extensions in the repository, and all of them are necessary:
ToSnakeCase on Enuminsupabase_common, extends a built-in type.GoTrueClientPasskey on GoTrueClientinsupabase_flutter, adds Flutter-only behavior to a class that lives in the pure Dartgotruepackage.GoTrueClientSignInProvider on GoTrueClient, same reason.Testing
dart analyzeand the fullstorage_clienttest suite pass. The existingTransformOptions.toQueryParamstests cover the getter unchanged.Summary by CodeRabbit