refactor!: apply Dart 3 class modifiers to the core public types - #1683
refactor!: apply Dart 3 class modifiers to the core public types#1683spydon wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughChangesThe PR applies Dart 3 class modifiers to public clients, builders, storage abstractions, authentication types, and utility classes. Production and test storage implementations now use Dart 3 class modifier migration
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 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 |
There was a problem hiding this comment.
Pull request overview
Applies Dart 3 class modifiers across public SDK types to formalize extension and implementation boundaries.
Changes:
- Adds
interface,abstract interface,base, andfinalmodifiers. - Updates affected test doubles and realtime tests.
- Documents migration requirements.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
MIGRATION.md |
Documents breaking modifier changes. |
packages/supabase/lib/src/supabase_client.dart |
Makes the client an interface class. |
packages/supabase_flutter/lib/src/local_storage.dart |
Defines storage extension interfaces. |
packages/supabase_flutter/lib/src/supabase_auth.dart |
Finalizes the internal auth helper. |
packages/supabase_flutter/test/widget_test_stubs.dart |
Updates storage test implementations. |
packages/storage_client/lib/src/storage_file_api.dart |
Makes file API implementable only. |
packages/storage_client/lib/src/storage_client.dart |
Makes storage client implementable only. |
packages/storage_client/lib/src/fetch.dart |
Finalizes the internal fetch helper. |
packages/realtime_client/lib/src/realtime_client.dart |
Makes realtime client implementable only. |
packages/realtime_client/test/channel_test.dart |
Replaces subclass stubs with mocks. |
packages/realtime_client/test/socket_test.dart |
Registers mock channels directly. |
packages/realtime_client/test/socket_test_stubs.dart |
Removes the subclass-based socket stub. |
packages/postgrest/lib/src/postgrest.dart |
Makes PostgREST client implementable only. |
packages/postgrest/lib/src/postgrest_builder.dart |
Restricts external builder extension. |
packages/gotrue/lib/src/gotrue_client.dart |
Makes auth client implementable only. |
packages/gotrue/lib/src/fetch.dart |
Finalizes the internal fetch helper. |
packages/gotrue/lib/src/types/auth_exception.dart |
Establishes the exception hierarchy modifiers. |
packages/gotrue/lib/src/types/auth_state.dart |
Finalizes auth state. |
packages/gotrue/lib/src/types/gotrue_async_storage.dart |
Defines an async storage interface. |
packages/gotrue/test/utils.dart |
Updates the storage test implementation. |
packages/functions_client/lib/src/functions_client.dart |
Makes functions client implementable only. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| /// * [SharedPreferencesLocalStorage], that implements SharedPreferences as | ||
| /// storage method | ||
| abstract class LocalStorage { | ||
| abstract interface class LocalStorage { |
The core public types now declare their subtyping intent instead of leaving every class open by default. The clients and the request builders are `interface class`, so they can no longer be extended but can still be implemented, which is what mockito and mocktail need: `SupabaseClient`, `GoTrueClient`, `PostgrestClient`, `PostgrestBuilder`, `RealtimeClient`, `FunctionsClient`, `SupabaseStorageClient` and `StorageFileApi`. `LocalStorage` and `GotrueAsyncStorage`, the two abstractions consumers plug their own storage into, are `abstract interface class`. `AuthState` is a `final class`, and the internal `GotrueFetch`, `Fetch` and `SupabaseAuth` helpers are `final class`. The `AuthException` hierarchy is deliberately left alone; it is being restructured separately. The realtime test stubs that extended `RealtimeClient` are gone. The setAuth channel tests use a mocktail mock of the socket, and the socket tests register their mock channels on `RealtimeClient.channels` directly instead of intercepting `channel()`.
f0d71b7 to
e61c3dc
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
MIGRATION.md (1)
330-334: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the
AuthExceptionhierarchy change
MIGRATION.mddoes not mention thebase classchange forAuthExceptionor thefinal classchanges for its subclasses. Add the breaking-change guidance and migration path.🤖 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 `@MIGRATION.md` around lines 330 - 334, Update the class-modifier migration section in MIGRATION.md to explicitly document that AuthException is now a base class and its subclasses are final classes. Describe the resulting restriction on extending or implementing these types and provide the migration path for consumers that currently subtype them.
🤖 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 `@MIGRATION.md`:
- Around line 350-367: Clarify the LoggingSupabaseClient example as partial
composition that cannot substitute for SupabaseClient, since it only forwards
from. State that callers requiring SupabaseClient must use a complete delegating
wrapper or retain the original client type.
---
Nitpick comments:
In `@MIGRATION.md`:
- Around line 330-334: Update the class-modifier migration section in
MIGRATION.md to explicitly document that AuthException is now a base class and
its subclasses are final classes. Describe the resulting restriction on
extending or implementing these types and provide the migration path for
consumers that currently subtype them.
🪄 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: 93366a1b-45bb-4ee8-a878-4f4cb53f2a8a
📒 Files selected for processing (1)
MIGRATION.md
| If you were extending one of these to change its behaviour, wrap it instead: | ||
|
|
||
| ```dart | ||
| // Before | ||
| class LoggingSupabaseClient extends SupabaseClient { | ||
| LoggingSupabaseClient(super.url, super.key); | ||
| } | ||
|
|
||
| // After | ||
| class LoggingSupabaseClient { | ||
| LoggingSupabaseClient(this._inner); | ||
|
|
||
| final SupabaseClient _inner; | ||
|
|
||
| SupabaseQueryBuilder from(String table) { | ||
| print('from($table)'); | ||
| return _inner.from(table); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- MIGRATION.md excerpt ---'
sed -n '320,390p' MIGRATION.md
printf '%s\n' '--- SupabaseClient declarations/usages ---'
rg -n --glob '*.dart' 'class SupabaseClient|abstract class SupabaseClient|interface class SupabaseClient|final class SupabaseClient|SupabaseClient\(' .
printf '%s\n' '--- wrapper example references ---'
rg -n 'LoggingSupabaseClient|wrap it instead|SupabaseClient' MIGRATION.mdRepository: supabase/supabase-flutter
Length of output: 5784
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- SupabaseClient public API ---'
sed -n '45,190p' packages/supabase/lib/src/supabase_client.dart
printf '%s\n' '--- Static contract check for the migration example ---'
python3 - <<'PY'
from pathlib import Path
import re
doc = Path("MIGRATION.md").read_text()
match = re.search(
r"class LoggingSupabaseClient \{\n(?P<body>.*?)\n\}",
doc,
re.S,
)
assert match, "LoggingSupabaseClient wrapper not found"
body = match.group("body")
methods = re.findall(r"^\s*(?:[A-Za-z_<>,?]+\s+)+([A-Za-z_]\w*)\s*\(", body, re.M)
fields = re.findall(r"^\s*final\s+[^;]+;", body, re.M)
print("wrapper_has_implements:", bool(re.search(r"\bimplements\s+SupabaseClient\b", match.group(0))))
print("wrapper_methods:", methods)
print("wrapper_fields:", fields)
PYRepository: supabase/supabase-flutter
Length of output: 5094
Clarify the wrapper’s type contract.
LoggingSupabaseClient does not implement SupabaseClient and forwards only from, so it cannot replace a SupabaseClient where that type is required. State that this is a partial composition example, or delegate the complete public API.
🤖 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 `@MIGRATION.md` around lines 350 - 367, Clarify the LoggingSupabaseClient
example as partial composition that cannot substitute for SupabaseClient, since
it only forwards from. State that callers requiring SupabaseClient must use a
complete delegating wrapper or retain the original client type.
Closes SDK-819.
Most public classes in the SDK were plain
class, so consumers could extend or implement any of them, including internal plumbing. This declares the intent explicitly with Dart 3 class modifiers.What changed
interface class— implementable, not extendableThe client entry points and the request builder base.
interface classcloses offextendswhile leavingimplementsopen, so the standard mocking idiom keeps compiling:SupabaseClient,GoTrueClient,PostgrestClient,PostgrestBuilder,RealtimeClient,FunctionsClient,SupabaseStorageClient,StorageFileApi.The ticket proposed
final classfor these.finalalso blocksimplements, which would break every consumer test suite that mocks a client with no escape hatch, sointerfaceis the narrower break that still delivers "not designed for extension".abstract interface class— the two consumer extension pointsLocalStorageandGotrueAsyncStorage. Both declare only abstract members, so callers change one word:final class— fully closedAuthState, plus the internalGotrueFetch,FetchandSupabaseAuthhelpers.Notes on the rest of the ticket's proposal
AuthExceptionhierarchy is deliberately untouched. The ticket asks forbase class AuthExceptionwithfinalsubclasses, but the exceptions are being restructured by a separate chain of PRs, so the modifiers belong there rather than here where they would just conflict.AuthChangeEventis already anenum, so the proposedsealed classtreatment is moot; exhaustive switches over auth events are unchanged.AuthStateis a single class rather than a hierarchy, so it isfinalrather thansealed.StorageBucketApiis not exported frompackage:storage_client, so consumers cannot name it and its modifier would have no public effect. Closing it would also forceSupabaseStorageClientto bebase/finaland lose mockability, so it stays a plain class.PostgrestQueryBuilderand friends) stay open.PostgrestQueryBuilderis extended across package boundaries bySupabaseQueryBuilder, and marking the chainbase/finalwould make the wholefrom(...)chain unmockable.Test changes
Two realtime test stubs extended
RealtimeClientand had to go:channel_test.dartnow use a mocktail mock of the socket, which is exactly the escape hatchinterface classpreserves.removeandsetAuthtests insocket_test.dartregister their mock channels onRealtimeClient.channelsdirectly rather than overridingchannel()to hand them back.The
LocalStorageandGotrueAsyncStoragetest stubs switched fromextendstoimplements.Verification
dart analyze packages: no issuesdart format: no changesmain(1754 symbols, no additions or removals), sosdk-compliance.yamlneeds no changesMIGRATION.mddocuments each modifier under the v2 to v3 section.Summary by CodeRabbit
Documentation
API Updates
Tests