refactor!: remove all deprecated APIs and dead public surface for v3 - #1661
Conversation
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR removes deprecated v3 APIs across GoTrue, PostgREST, Realtime, Storage, and Supabase. It requires Changesv3 API cleanup
Estimated code review effort: 3 (Moderate) | ~25 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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/gotrue/lib/src/types/user.dart (1)
72-72: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve legacy confirmation data during session restore.
User.fromJsonnow ignoresconfirmed_at, andUser.toJsonno longer writes it. A v2-persisted user payload can containconfirmed_atwithoutemail_confirmed_at. Restoring that payload then setsUser.emailConfirmedAttonull.If v2 session payloads remain restorable, read
email_confirmed_atfirst and fall back toconfirmed_at. Continue writing onlyemail_confirmed_at. Add a regression test.As per coding guidelines, add or maintain tests for modified package behavior.
Proposed compatibility fix
- emailConfirmedAt: json['email_confirmed_at'], + emailConfirmedAt: + json['email_confirmed_at'] ?? json['confirmed_at'],#!/bin/bash set -euo pipefail rg -n -C 5 --glob '*.dart' \ "confirmed_at|email_confirmed_at|User\.fromJson|User\.toJson|Session" \ packages/gotrue packages/supabase packages/supabase_flutterAlso applies to: 104-104
🤖 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/gotrue/lib/src/types/user.dart` at line 72, Update User.fromJson to populate emailConfirmedAt from email_confirmed_at first, falling back to confirmed_at when the new field is absent, while leaving User.toJson writing only email_confirmed_at. Add a regression test covering restoration of a legacy payload containing confirmed_at without email_confirmed_at.
🧹 Nitpick comments (1)
packages/gotrue/test/src/constants_test.dart (1)
87-92: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a regression test for the removed event name.
The change removes the positive
userDeletedassertions, but it does not verify that the old serialized name is rejected. Addexpect(AuthChangeEvent.fromString('userDeleted'), isNull)to the invalid-name test.As per coding guidelines, maintain tests for modified package behavior and run backend-dependent GoTrue tests with
dart test -j 1.Proposed test addition
test('fromString returns null for invalid names', () { expect(AuthChangeEvent.fromString('invalid'), isNull); + expect(AuthChangeEvent.fromString('userDeleted'), isNull); expect(AuthChangeEvent.fromString('SIGNED_IN'), isNull);Also applies to: 119-123
🤖 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/gotrue/test/src/constants_test.dart` around lines 87 - 92, Update the invalid-name test for AuthChangeEvent to assert that AuthChangeEvent.fromString('userDeleted') returns null, covering rejection of the removed serialized event name. Keep the existing invalid-name assertions intact and run the backend-dependent GoTrue tests with dart test -j 1.Source: Coding guidelines
🤖 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 118-119: Update the migration introduction around the v3
deprecated-surface statement to limit the claim to removed APIs that have
documented replacements, removing the assertion that every entry has one. Keep
the surrounding deprecation and version context unchanged.
In `@packages/postgrest/lib/src/postgrest.dart`:
- Around line 87-88: Update setAuth to remove every authorization header key
case-insensitively before applying or clearing the token, including clients
initialized with differently cased keys; preserve the documented null-clears
behavior and add a regression test covering this case. Run the PostgREST package
tests sequentially with dart test -j 1.
---
Outside diff comments:
In `@packages/gotrue/lib/src/types/user.dart`:
- Line 72: Update User.fromJson to populate emailConfirmedAt from
email_confirmed_at first, falling back to confirmed_at when the new field is
absent, while leaving User.toJson writing only email_confirmed_at. Add a
regression test covering restoration of a legacy payload containing confirmed_at
without email_confirmed_at.
---
Nitpick comments:
In `@packages/gotrue/test/src/constants_test.dart`:
- Around line 87-92: Update the invalid-name test for AuthChangeEvent to assert
that AuthChangeEvent.fromString('userDeleted') returns null, covering rejection
of the removed serialized event name. Keep the existing invalid-name assertions
intact and run the backend-dependent GoTrue tests with dart test -j 1.
🪄 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: dc24978b-bc51-42da-95b4-bf10016df85e
📒 Files selected for processing (30)
MIGRATION.mdpackages/gotrue/lib/src/constants.dartpackages/gotrue/lib/src/types/types.dartpackages/gotrue/lib/src/types/user.dartpackages/gotrue/test/custom_oauth_provider_test.dartpackages/gotrue/test/src/constants_test.dartpackages/gotrue/test/src/types/user_test.dartpackages/postgrest/lib/src/postgrest.dartpackages/postgrest/lib/src/types.dartpackages/realtime_client/lib/src/constants.dartpackages/realtime_client/lib/src/push.dartpackages/realtime_client/lib/src/realtime_client.dartpackages/realtime_client/lib/src/types.dartpackages/realtime_client/test/channel_test.dartpackages/realtime_client/test/mock_test.dartpackages/realtime_client/test/socket_test.dartpackages/storage_client/lib/src/storage_file_api.dartpackages/storage_client/lib/src/types.dartpackages/storage_client/test/basic_test.dartpackages/supabase/lib/src/auth_user.dartpackages/supabase/lib/src/realtime_client_options.dartpackages/supabase/lib/src/remove_subscription_result.dartpackages/supabase/lib/src/supabase_event_types.dartpackages/supabase/lib/src/supabase_realtime_error.dartpackages/supabase/lib/src/supabase_stream_builder.dartpackages/supabase/lib/supabase.dartpackages/supabase/test/mock_test.dartpackages/supabase/test/utilities_test.dartpackages/supabase_flutter/lib/src/supabase.dartpackages/yet_another_json_isolate/lib/yet_another_json_isolate.dart
💤 Files with no reviewable changes (14)
- packages/supabase/lib/src/auth_user.dart
- packages/supabase/lib/src/supabase_stream_builder.dart
- packages/supabase/lib/src/supabase_event_types.dart
- packages/supabase/lib/src/supabase_realtime_error.dart
- packages/gotrue/lib/src/constants.dart
- packages/supabase/test/mock_test.dart
- packages/gotrue/lib/src/types/types.dart
- packages/gotrue/test/src/types/user_test.dart
- packages/postgrest/lib/src/types.dart
- packages/supabase/lib/src/remove_subscription_result.dart
- packages/gotrue/test/custom_oauth_provider_test.dart
- packages/supabase/lib/supabase.dart
- packages/supabase/lib/src/realtime_client_options.dart
- packages/supabase/test/utilities_test.dart
Drops every `@Deprecated` declaration across the packages, plus two pieces of dead surface that were never marked but have no reason to survive a major version: - `Transports` held a single `websocket` string used at one call site, now inlined. - The `dart.library.js` export fallback in `yet_another_json_isolate` targeted Dart below 3.3, while every package requires `>=3.9.0`. `createSignedUrlsResult` takes over the `createSignedUrls` name now that the overload that silently dropped unsignable paths is gone, and `publishableKey` becomes required on `Supabase.initialize`.
b0f853e to
bea73e3
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Also narrows the MIGRATION.md claim that every removed API has a replacement, which is not true of the entries that were already inert.
Closes SDK-1428. Part of #1278.
Removes every
@Deprecateddeclaration across the packages, plus two pieces of dead public surface that were never marked but have no reason to survive a major version.Deprecated APIs removed
AuthChangeEvent.userDeletedgotrueOAuthProvider.snakeCaseOAuthProvider.namegotrueUser.confirmedAtUser.emailConfirmedAtgotrueReturningOptionpostgrestPostgrestClient.auth()PostgrestClient.setAuth()postgrestRealtimeClient.longpollerTimeoutrealtime_clientChannelResponse.rateLimitedrealtime_clientFileObject.lastAccessedAt,FileObjectV2.lastAccessedAtstorage_clientcreateSignedUrlsreturningList<SignedUrl>createSignedUrlsreturningList<SignedUrlResult>storage_clientAuthUserUsersupabaseRealtimeClientOptions.eventsPerSecondsupabaseRemoveSubscriptionResultsupabaseSupabaseRealtimeErrorsupabaseSupabaseEventTypesandSupabaseEventTypesNamesupabaseSupabaseStreamBuilder.execute()supabaseSupabase.initialize(anonKey:)Supabase.initialize(publishableKey:), now requiredsupabase_flutterThree of these are more than a field going away.
createSignedUrlsResulttakes over thecreateSignedUrlsname (SDK-1002). Dart has no overloading, so the version that reports per-path failures had to ship under a temporary name while the old one that silently dropped unsignable paths was still around. With the old one gone the temporary name can go too.publishableKeybecomes a required named parameter onSupabase.initialize, which also removes the runtime assertion that one of the two keys was passed. A legacy anon key is still a valid value, it just goes under the new name.Both
lastAccessedAtfields were required constructor parameters, so code that builds aFileObjectorFileObjectV2by hand drops that argument.User.confirmedAtis likewise no longer parsed from or written to JSON, sotoJson()output no longer carries aconfirmed_atkey.Not deprecated, but removed too
A pass over the public surface turned up two things worth taking with them:
Transportsheld a singlewebsocketstring constant used at exactly one call site, a leftover of thephoenix/realtime-jsport. Inlined at the call site.dart.library.jsconditional export fallback inyet_another_json_isolateexisted for Dart below 3.3, while every package in the repo already requires>=3.9.0.I also considered renaming
ConstantstoRealtimeConstantsinrealtime_clientand dropping thetypedef RealtimeConstants = Constantsthat fronts it. I left it alone:RealtimeConstantsis already the only name exported, so the rename buys consumers nothing, while the symbol extractor treats all five of its static members as new public API and would force registering constants likewsCloseNormalanddefaultHeadersinsdk-compliance.yamlas if they were capabilities. Not worth the matrix churn for an invisible rename, and it deserves its own decision if anyone wants it.Larger reshapes that have their own tickets are deliberately left alone: SDK-818 (private
RealtimeClientfields, which is also where the stringly-keyedstateChangeCallbacksmap belongs), SDK-817 (DateTimeinstead ofStringtimestamps), SDK-1287 and SDK-1313 (retry config), SDK-820 and SDK-1289 (typed rows).Notes
MIGRATION.mdgets a section per removal under v2 to v3, with before/after snippets for the ones that need more than a rename.sdk-compliance.yamlneeds no change. Its only affected entry isStorageFileApi.createSignedUrls, which keeps its name, and every other removed symbol was unregistered.Testing
dart analyze --fatal-infosis clean across the workspace, as isflutter analyzeon the examples. Both capability-matrix checks pass locally againstorigin/mainas the base (check-api-symbolsandcheck-drift). Every package's tests pass against a local Supabase stack:gotrue(460),postgrest(196),storage_client(210),realtime_client(203),supabase(134),supabase_flutter(63),functions_client(48),yet_another_json_isolate(22).Summary by CodeRabbit
Supabase.initializenow requires apublishableKey.createSignedUrls.nulltosetAuthclears the authorization header.