chore: mark non-public declarations @internal - #1671
Conversation
The capability matrix extractor treats every non-underscore name in lib/ as public API, without following exports. Plenty of implementation plumbing in lib/src/ therefore counted as public surface that had to be either registered in sdk-compliance.yaml or left as an unexplained gap. @internal is the marker the extractor already honours for exactly this case, and the repo already uses it on members; this extends it to the declarations themselves. Annotates 24 declarations that are public by Dart's underscore rule but are not reachable from their package's public library: HTTP plumbing (GotrueFetch, Fetch, AuthHttpClient, GotrueRequestOptions, RequestMethodType, ApiVersion), realtime transport internals (Push, Hook, Message, Serializer, RetryTimer and their typedefs), per-package Constants, and a few helpers (Counter, ToQueryParams, SupabaseAuth, the admin OAuth client response wrappers). Deliberately excluded: anything with symbols registered in sdk-compliance.yaml as capability evidence, since annotating those would hide them from the extractor and break the drift check. That ruled out StorageBucketApi, GoTrueAdminOAuthApi, GoTrueAdminCustomProvidersApi, GoTrueAdminMFAApi, realtime's Constants and ChannelFilter. Also excluded File and YAJsonIsolate, which are reachable publicly via conditional imports and exports. functions_client gains a meta dependency, which it did not previously have. Reported public surface drops from 1952 symbols to 1831.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (19)
📝 WalkthroughWalkthroughThe PR adds ChangesInternal API visibility
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
This PR reduces the reported public API surface across the Supabase Flutter/Dart monorepo by marking implementation-only declarations in lib/src/ as @internal, so the capability-matrix extractor no longer treats them as supported public API.
Changes:
- Annotated internal-only classes/typedefs/extensions in multiple packages with
@internal(and addedpackage:meta/meta.dartimports where needed). - Added a direct
metadependency tofunctions_clientto support the new@internalusage there. - Kept analyzer legality intact (no
invalid_internal_annotation/ cross-packageinvalid_use_of_internal_memberfallout per the PR test plan).
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/supabase/lib/src/counter.dart | Marks Counter as internal API via @internal. |
| packages/supabase/lib/src/constants.dart | Marks Constants as internal API via @internal. |
| packages/supabase/lib/src/auth_http_client.dart | Marks AuthHttpClient as internal API via @internal. |
| packages/supabase_flutter/lib/src/supabase_auth.dart | Marks SupabaseAuth implementation plumbing as @internal. |
| packages/supabase_flutter/lib/src/constants.dart | Marks Constants as internal API via @internal. |
| packages/storage_client/lib/src/types.dart | Marks ToQueryParams extension as @internal. |
| packages/storage_client/lib/src/fetch.dart | Marks Fetch as internal API via @internal. |
| packages/storage_client/lib/src/constants.dart | Marks Constants as internal API via @internal. |
| packages/realtime_client/lib/src/serializer.dart | Marks Serializer as internal API via @internal. |
| packages/realtime_client/lib/src/retry_timer.dart | Marks reconnect timer typedefs + RetryTimer as @internal. |
| packages/realtime_client/lib/src/push.dart | Marks Callback, Push, and Hook as @internal. |
| packages/realtime_client/lib/src/message.dart | Marks Message as internal API via @internal. |
| packages/gotrue/lib/src/types/fetch_options.dart | Marks GotrueRequestOptions as @internal. |
| packages/gotrue/lib/src/types/api_version.dart | Marks ApiVersion as internal API via @internal. |
| packages/gotrue/lib/src/gotrue_admin_oauth_api.dart | Marks OAuth admin response types as @internal. |
| packages/gotrue/lib/src/fetch.dart | Marks RequestMethodType and GotrueFetch as @internal. |
| packages/gotrue/lib/src/constants.dart | Marks Constants as internal API via @internal. |
| packages/functions_client/pubspec.yaml | Adds meta dependency to support @internal. |
| packages/functions_client/lib/src/constants.dart | Marks Constants as internal API via @internal. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Stacked on #1673. Follow-up to the audit in that PR: of the 536 entries in the top-level `supporting_symbols` list, ten were realtime transport internals that had no business being public in the first place. They were registered there for want of a better home, which was the wrong answer. ## What becomes `@internal` | Symbol | What it is | |---|---| | `RealtimeClient.sendBuffer` | Queue of callbacks awaiting a connection | | `RealtimeClient.stateChangeCallbacks` | The `open`/`close`/`error`/`message` listener registry behind `onOpen` and friends | | `RealtimeClient.ref` | Message ref counter | | `RealtimeClient.makeRef` | Increments the counter, handling overflow | | `RealtimeClient.push` | Raw frame send | | `RealtimeClient.heartbeatTimer` | The `Timer` driving heartbeats | | `RealtimeClient.pendingHeartbeatRef` | Ref of the last unacknowledged heartbeat | | `RealtimeClient.reconnectTimer` | The `RetryTimer` behind reconnect backoff | | `RealtimeChannel.canPush` | Whether the socket is connected and the channel joined | | `RealtimeChannel.trigger` | Dispatches a raw event to the channel's bindings | `push` is the clearest case: it already took `Message`, which #1671 marked `@internal`, so it could not be called from outside the package regardless. Its signature said internal while its name said public. The rest are fields on a class that happens to be public, so Dart's underscore rule made them public by default. None appears in any documented flow. Every one of them has a public counterpart that is the supported way in: `onOpen`/`onClose`/`onError`/`onMessage` instead of `stateChangeCallbacks`, `heartbeatIntervalMs` instead of `heartbeatTimer`, `reconnectAfterMs` instead of `reconnectTimer`, `sendBroadcastMessage` instead of `push`. ## Matrix changes Ten symbols leave the scan, so they leave `sdk-compliance.yaml` too: - Eight drop out of the top-level `supporting_symbols` list. - `RealtimeClient.reconnectTimer` drops from `realtime.configuration.reconnect_backoff`, and `heartbeatTimer`/`pendingHeartbeatRef` from `realtime.configuration.heartbeat_interval`. Both features keep their real evidence (`reconnectAfterMs`, `heartbeatIntervalMs`), and `reconnect_backoff` no longer needs a `supporting_symbols` list at all. Public surface 1774 → 1764. Coverage stays at 100%. ## Breaking Marked `chore(realtime)!` with a `BREAKING CHANGE` footer. It is a genuine public API removal, though the practical risk is low: none of these is documented, `push` was already uncallable, and v3 is the right window. Anyone reaching for them was working around a missing public API, which is worth an issue rather than a lint suppression. ## Test plan - [x] `dart analyze packages/`: **No issues found.** This is the gate that matters here: `invalid_use_of_internal_member` fires for any cross-package use, and the `supabase` and `supabase_flutter` packages both consume `RealtimeClient` and `RealtimeChannel`. Neither touches any of the ten. - [x] `realtime_client` tests: 205 passed. `@internal` permits same-package use, so the existing tests that drive `makeRef`, `push` and `trigger` still compile and run. - [x] `supabase` tests: 134 passed, including the realtime stream integration tests. - [x] `supabase_flutter` tests: 65 passed. - [x] `check-drift`: `✅ No capability matrix drift detected.` - [x] `check-api-symbols` against #1673 as base: `✅ All new public API symbols are covered in the capability matrix.` This is the check that would have caught a stale registration, since the ten removed symbols were all registered on the base. - [x] Uncovered symbol count from a fresh extraction: **0**. - [x] `dart format packages/`: 0 changed.
) Stacked on #1671. Closes the backfill that #1670 and #1671 flagged: capability-matrix coverage of the Dart public API goes from **49% to 100%**. | | before | after | |---|---|---| | Symbols reported as public API | 1831 | **1774** | | Registered | 899 | **1766** | | Unregistered | 930 | **0** | ## How the 881 unregistered symbols were placed **26 are entry points the matrix was missing outright.** These go in `symbols`, so the drift check now verifies them: | Feature | Entry point | |---|---| | `auth.session.update_user` | `GoTrueClient.updateUser` | | `auth.session.get_session` / `get_user` | `GoTrueClient.currentSession` / `currentUser` | | `auth.sign_in.sign_in_with_oauth` | `GoTrueClient.getOAuthSignInUrl`, `GoTrueClientSignInProvider.signInWithOAuth` | | `auth.sign_in.sign_in_with_sso` | `GoTrueClientSignInProvider.signInWithSSO` | | `auth.identities.link_identity` | `GoTrueClientSignInProvider.linkIdentity` | | `auth.passkey.*` | `GoTruePasskeyApi.start/verifyRegistration`, `start/verifyAuthentication` | | `database.query.from_table` / `rpc` / `schema_selection` | `SupabaseClient.from` / `rpc` / `schema` | | `database.using_modifiers.explain` | `PostgrestTransformBuilder.explain` | | `realtime.subscriptions.postgres_changes` | `RealtimeChannel.onPostgresChanges`, `onSystemEvents` | | `realtime.client.*` | `SupabaseClient.channel`, `getChannels`, `removeChannel`, `removeAllChannels`, `RealtimeClient.disconnect` | | `storage.file_buckets.create_signed_url` | `StorageFileApi.createSignedUrl` | | `client.*` | `GoTrueClient.getSessionFromUrl`, `recoverSession`, `setInitialSession` | Several of these features previously listed only an option or enum as their evidence. `storage.file_buckets.create_signed_url` claimed `DownloadBehavior`; `database.using_modifiers.explain` claimed `ExplainFormat`; `auth.session.update_user` claimed `UserAttributes.currentPassword`. The method that actually implements the capability was unregistered in each case. **500 go in a feature's `supporting_symbols`**, where the type maps onto one capability: the MFA response types to their operations, `Jwt*`/`JWK*`/`DecodedJwt` to `get_claims`, `FileObjectV2` to `file_info`, `SignedUrl*` to `create_signed_urls`, the presence payloads to `subscribe_presence`, and so on. **355 go in the top-level `supporting_symbols` list.** Four kinds: - **Shared domain models** — `Session`, `User`, `AuthResponse`, `Bucket`, `OAuthClient`, `Factor`. Accepted or returned by many features at once, so no single feature id is a truthful home. - **Exception hierarchies** — `AuthException`, `PostgrestException`, `FunctionException`, `RealtimeSubscribeException` and subclasses. Thrown rather than passed, so reachable from no signature. - **Client and sub-API handles** — `SupabaseClient`, `GoTrueClient`, `RealtimeClient`, `GoTrueAdminApi` and the accessors returning them gate whole areas; their operations are attributed individually. - **Surface with no canonical id** — see the last section. ## 26 symbols left the scan instead of entering the matrix #1671's script only matched class-like declarations, so top-level functions and variables slipped through. Now `@internal`: - `realtime_client/src/transformers.dart`: `convertCell`, `convertColumn`, `convertChangeData`, `toArray`, `toBoolean`, `toDouble`, `toInt`, `toJson`, `toTimestampString`, `noop`, `httpEndpointURL`, `getEnrichedPayload`, `getPayloadRecords` - conditional-import platform shims: `accessToken`, `hasAccessToken`, `persistSession`, `removePersistedSession`, `disposePreviousClient`, `markClientToDispose`, `supabaseFlutterClientToDispose`, `getBroadcastChannel`, `createWebSocketClient` - `passkeyRegisterRequestFromOptions`, `passkeyAuthenticateRequestFromOptions`, `maxShift`, `defaultHeaders` `ChannelFilter` joins them, and this one is worth a second look: it is hidden from `realtime_client`'s public library, so `ChannelFilter.select` could never have been drift-verified evidence for `realtime.subscriptions.postgres_changes`. `RealtimeChannel.onPostgresChanges` replaces it. Two paths cannot carry the annotation, so `.sdk-parse-ignore` excludes them: - `packages/*/example/` — each package ships its own example app, and the extractor treats it as a package of its own because it has a `pubspec.yaml`. `MyApp`, `MyWidget` and `main` were counted as SDK public API. `examples/` was already excluded for exactly this reason. - `packages/*/lib/src/version.dart` — the release tooling rewrites it wholesale (`echo "const version = '$version';" > ...`), so an annotation would not survive a release. ## One behaviour-visible change `realtime_client.dart` changes from `export 'src/transformers.dart' hide getEnrichedPayload, getPayloadRecords` to `show PostgresColumn, PostgresType`. The 13 payload-conversion helpers above were public only by accident of that hide list, and the analyzer requires it: `@internal` on an exported member is `invalid_export_of_internal_element`. The `show` form also stops the next helper added to that file from leaking. This removes public API. It is not API anyone should be calling (`toInt`, `noop`, `convertCell`), and v3 is already removing dead public surface, but it is the one item here that is not purely additive. ## Also declared: `storage.errors.error_codes` The validator reported it undeclared, and `StorageException.error` already carries the machine-readable service code the capability describes. Declared as implemented, which also gives `StorageException` a real home instead of the shared bucket. ## Gaps this surfaced Public API with no canonical capability id, currently parked in the top-level list. Each is a candidate for a new id in `supabase/sdk`: 1. **Client construction and disposal** — `Supabase.initialize`, `Supabase.instance`, `Supabase.client`, `isInitialized`, and `dispose` on every client. `client.*` has no lifecycle group at all. 2. **User-facing passkey management** — `GoTruePasskeyApi.list`, `delete`, `update`. The matrix has `auth.passkey_admin.list_passkeys` and `delete_passkey` for the admin API, but nothing for a user managing their own. 3. **Realtime streams as a database capability** — `SupabaseQueryBuilder.stream` and `SupabaseStreamBuilder`. The stream *filters* and *modifiers* are registered under `database.using_filters.*` and `using_modifiers.*`, but `stream()` itself is not a capability. 4. **Storage retry configuration** — `StorageClientOptions.retryAttempts` and `StorageRetryController`. `database.configuration.auto_retry` exists; there is no storage equivalent. 5. **`realtime.subscriptions.postgres_changes_multiple_filters`** is the one remaining undeclared feature, and Dart implements it (`onPostgresChanges(filters: [...])`). Declaring it needs `RealtimeChannel.onPostgresChanges` registered against two features, so it is left out here rather than adding a duplicate registration. Two more judgement calls worth flagging: - **`RealtimeClient` exposes transport internals** — `sendBuffer`, `stateChangeCallbacks`, `pendingHeartbeatRef`, `makeRef`, `ref`, `push`, `heartbeatTimer`, `reconnectTimer`, and `RealtimeChannel.canPush`/`trigger`. Registered here because they are genuinely exported, but they read like `@internal` candidates for a future breaking change. - **`yet_another_json_isolate`** contributes 13 symbols with no Supabase capability behind them. Registered in the top-level list; it could instead be excluded from the scan the way `supabase_common` is. ## Test plan - [x] `dart analyze packages/`: **No issues found.** This is the real gate for `invalid_internal_annotation`, `invalid_use_of_internal_member` and `invalid_export_of_internal_element`. - [x] Compliance validator: `OK — compliance file is valid.` - [x] `check-drift`: `✅ No capability matrix drift detected.` - [x] `check-api-symbols` against #1671 as base: `✅ All new public API symbols are covered in the capability matrix.` Nothing registered was removed, and nothing new is uncovered. - [x] Uncovered symbol count recomputed from a fresh extraction: **0**. - [x] Checked for symbols registered in more than one place: 9, all pre-existing and deliberate (one method serving two capabilities, e.g. `StorageFileApi.move` for `move` and `move_cross_bucket`). This change adds none. - [x] `dart format packages/`: 0 changed. - [x] Tests, all passing: `gotrue` 471, `realtime_client` 205, `postgrest` 196, `supabase` 134, `storage_client` 210, `supabase_flutter` 65, `functions_client` 48. `postgrest` needs `-j 1`; run in parallel its test files race on the shared database reset helper, which varies the failure count run to run on unmodified code. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **API Improvements** - Clarified which lower-level SDK helpers are intended for internal use, improving API guidance and tooling. - Explicitly exposed realtime PostgreSQL column and type definitions for supported integrations. - **Compatibility** - Expanded SDK capability validation across authentication, database, storage, realtime, functions, persistence, passkeys, and client features. - Added broader coverage for public models, builders, responses, errors, enums, and utility types. - **Bug Fixes** - Improved type safety for persisted session data handling. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
The capability matrix extractor treats every non-underscore name in
lib/as public API, without following exports:So implementation plumbing in
lib/src/counted as public surface that had to be either registered insdk-compliance.yamlor left as an unexplained gap. This marks that plumbing@internal. The repo already uses the annotation on individual members; this extends it to the declarations themselves.Annotates 24 declarations:
realtime_clientPush,Hook,Message,Serializer,RetryTimer,Callback,TimerCallback,TimerCalculationgotrueGotrueFetch,GotrueRequestOptions,RequestMethodType,ApiVersion,Constants,OAuthClientResponse,OAuthClientListResponsestorage_clientFetch,ToQueryParams,ConstantssupabaseAuthHttpClient,Counter,Constantssupabase_flutterSupabaseAuth,Constantsfunctions_clientConstantsHow the set was chosen
Not by eye. A declaration qualifies only if it is unreachable from its package's public library, resolving
exportdirectives transitively and honouringshow/hide. That makes the annotation truthful rather than a judgement call, and keeps it analyzer-legal, since@internalon something in the public API is itself a diagnostic.Candidates were then filtered against
sdk-compliance.yaml. Annotating a class hides its members from the extractor too, so anything with registered symbols would silently break the drift check. That filter removed six:storage_client/StorageBucketApicreateBucket,listBuckets, ...)gotrue/GoTrueAdminCustomProvidersApigotrue/GoTrueAdminOAuthApigotrue/GoTrueAdminMFAApirealtime_client/ConstantsdefaultConnectionCloseTimeout)realtime_client/ChannelFilterselect)Two more were excluded for being publicly reachable in ways a naive export scan misses:
storage_client/Fileis a conditional-import typedef used inStorageFileApi.uploadsignatures, so it is effectively public.yet_another_json_isolate/YAJsonIsolatelives in_isolates_web.dartbut is conditionally exported (if (dart.library.js_interop)), so it is the package's public entry point.Effect
111 symbols stop being counted as public API. No symbol newly appears, and no registered symbol disappears.
This shrinks the backfill problem noted in #1670 by about a ninth. The remaining 930 are genuine public API that is simply unregistered; that is a separate task.
Test plan
dart analyze packages/: No issues found. This is the real gate: the analyzer reportsinvalid_internal_annotationfor anything annotated inside a public API, andinvalid_use_of_internal_memberfor cross-package use. Neither fires.Constants, which each package declares for itself; verifiedsupabase_flutter/src/supabase.dartimports its ownsrc/constants.dart, so that use is same-package.check-driftagainst a freshly extracted surface:✅ No capability matrix drift detected.dart format packages/: 0 changed.gotruetests:+448 -23, identical to unmodifiedmain(+448 -23). The 23 failures are integration tests needing a local GoTrue onlocalhost:9999and are unrelated to this change, which is annotation-only and cannot affect runtime behaviour.Note
functions_clientgains ameta: ^1.16.0dependency, which it did not previously have. Every other touched package already depended on it.Summary by CodeRabbit