fix(supabase_flutter)!: persist the session with SharedPreferencesAsync - #1680
Merged
Conversation
The default storage implementations wrote through the legacy SharedPreferences API. Mixing that API with SharedPreferencesAsync in one app drops values on some platforms, so an app that had migrated its own code to the new API lost the persisted session. SharedPreferencesLocalStorage and SharedPreferencesGotrueAsyncStorage now use SharedPreferencesAsync, and initialize() moves a session written by v2 over to the new store so users stay signed in. Closes #1276
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change migrates Flutter session persistence to ChangesSession persistence migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant SharedPreferencesLocalStorage
participant SharedPreferencesAsync
participant SharedPreferences
SharedPreferencesLocalStorage->>SharedPreferencesAsync: Read current session
SharedPreferencesLocalStorage->>SharedPreferences: Read legacy session
SharedPreferences->>SharedPreferences: Remove migrated entry
SharedPreferencesLocalStorage->>SharedPreferencesAsync: Write migrated session
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
MIGRATION.md (1)
389-389: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueWrap the Dart example to the configured line length.
Line 389 exceeds 80 characters. Wrap the
persistSessionKeyargument asdart formatwould.As per coding guidelines, Dart code must use
dart formatand an 80-character line length.🤖 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` at line 389, Wrap the persistSessionKey argument in the Dart example to comply with dart format’s 80-character line length, preserving the existing expression and behavior.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 `@packages/supabase_flutter/lib/src/local_storage.dart`:
- Around line 92-106: Update _migrateLegacySession so it removes the legacy
persistSessionKey from SharedPreferences before returning when _preferences
already contains the key. Add a regression test that seeds both stores, calls
removePersistedSession(), recreates the storage, and verifies no session is
restored.
In `@packages/supabase_flutter/README.md`:
- Line 507: Update the opening phrase in the session persistence documentation
from “As default” to “By default,” leaving the remainder of the guidance
unchanged.
---
Nitpick comments:
In `@MIGRATION.md`:
- Line 389: Wrap the persistSessionKey argument in the Dart example to comply
with dart format’s 80-character line length, preserving the existing expression
and behavior.
🪄 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: 19a2762c-5456-4815-b829-23aaec258860
📒 Files selected for processing (10)
AGENTS.mdMIGRATION.mdpackages/supabase_flutter/README.mdpackages/supabase_flutter/lib/src/local_storage.dartpackages/supabase_flutter/pubspec.yamlpackages/supabase_flutter/test/deep_link_test.dartpackages/supabase_flutter/test/dispose_test.dartpackages/supabase_flutter/test/initialization_test.dartpackages/supabase_flutter/test/storage_test.dartpackages/supabase_flutter/test/utils.dart
…web file The migration only runs on the platforms that store the session through shared_preferences. On web the session lives in window.localStorage under the same key as it did in v2, so there is nothing to migrate and the tests failed there.
Contributor
There was a problem hiding this comment.
Pull request overview
Migrates Supabase Flutter session and PKCE persistence to SharedPreferencesAsync, including legacy-session migration.
Changes:
- Replaces legacy preference storage with the asynchronous API.
- Adds migration logic and regression tests.
- Documents the breaking change and migration options.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
AGENTS.md |
Updates storage architecture guidance. |
MIGRATION.md |
Documents migration and legacy-storage fallback. |
packages/supabase_flutter/README.md |
Updates session-storage guidance. |
packages/supabase_flutter/pubspec.yaml |
Adds the test platform-interface dependency. |
packages/supabase_flutter/lib/src/local_storage.dart |
Implements asynchronous storage and legacy migration. |
packages/supabase_flutter/test/utils.dart |
Adds shared-preferences test mocks. |
packages/supabase_flutter/test/storage_test.dart |
Updates storage tests for the asynchronous API. |
packages/supabase_flutter/test/local_storage_migration_test.dart |
Tests legacy-session migration scenarios. |
packages/supabase_flutter/test/initialization_test.dart |
Uses the shared test mock. |
packages/supabase_flutter/test/dispose_test.dart |
Uses the shared test mock. |
packages/supabase_flutter/test/deep_link_test.dart |
Verifies persistence through the asynchronous API. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Deleting the legacy entry does not make the move one-time. On the platforms where both shared_preferences APIs rewrite one file from their own cache, a later write through either API can bring the deleted entry back, and the delete was skipped altogether when the new store already had a session. In both cases the next launch after a sign-out restored the stale session and signed the user back in. A key in the new store now records that the move happened, so the legacy store is read at most once. The legacy entry is still deleted, but only so that a stale token does not lie around.
… platform The guide framed the collision as the session going missing, which is the v2 direction. From v3 on it runs the other way as well: on Windows and Linux a session write by the SDK can drop preferences the app wrote through the legacy API, which is the risk raised on the superseded pull request.
Contributor
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/supabase_flutter/lib/src/local_storage.dart`:
- Around line 102-109: Update _migrateLegacySession so it obtains and reloads
SharedPreferences before checking _legacyMigrationKey, then removes
persistSessionKey when the migration marker already exists before returning.
Preserve the existing migration behavior for unmarked stores, and extend the
relevant local storage migration test to verify a recreated legacy session is
removed.
🪄 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: ec980533-c7b4-4723-9a8b-724b7e3f4078
📒 Files selected for processing (4)
MIGRATION.mdpackages/supabase_flutter/README.mdpackages/supabase_flutter/lib/src/local_storage.dartpackages/supabase_flutter/test/local_storage_migration_test.dart
🚧 Files skipped from review as they are similar to previous changes (2)
- MIGRATION.md
- packages/supabase_flutter/README.md
…SessionKey (#1681) Stacked on #1680, review that one first. ## What kind of change does this PR introduce? Cleanup and a small feature, breaking change. ## What is the current behavior? `supabase_flutter` still carries the leftovers of the v1 to v2 migration: - `supabasePersistSessionKey`, a public constant whose own dartdoc says "Only used for migration from Hive to SharedPreferences. Not actually in use." The SDK never reads it. It is only the key v1 stored the session under. - A README section with `MigrationLocalStorage` and `HiveLocalStorage` snippets to copy, which move a session out of [hive](https://pub.dev/packages/hive) on the first launch after upgrading from v1. The README's `MySecureStorage` example used the constant as its storage key, which was wrong for anything but a v1 leftover: the session lives under the key that is passed to `LocalStorage`, `sb-<project-ref>-auth-token` for the default storage. That key was derived in two places (`Supabase.initialize` and, for the broadcast channel name, `GoTrueClient`) with nothing public to call, so a custom `LocalStorage` had no way to name the same key other than writing the format out by hand. ## What is the new behavior? Both leftovers are gone. Anyone still on v1 can upgrade to v2, let it migrate the session, and then move to v3. `defaultPersistSessionKey(url)` replaces the hand-written format. It lives in `supabase_common`, which gotrue and supabase_flutter both already depend on, and `supabase_flutter` exports it the way `postgrest` and `functions_client` export `HttpMethod`: ```dart authOptions: FlutterAuthClientOptions( localStorage: MySecureStorage( persistSessionKey: defaultPersistSessionKey(supabaseUrl), ), ), ``` The derivation now exists in exactly one place, and both call sites use it. Nothing changes for the default path: `Supabase.initialize` still fills the key in, so `Supabase.initialize(url:, publishableKey:)` remains all a caller writes. The `Migrating Guide` section of the README pointed at the v1 to v2 upgrade guide on the docs site, and now points at `MIGRATION.md`, which is where the v2 to v3 changes are written down. Both docs also say explicitly that the key is derived for you and only needs naming when you supply your own storage, which was easy to misread before. ## Additional context `MIGRATION.md` documents the removal, including the literal value of the constant for anyone who copied the old example and needs to keep reading the same key. `sdk-compliance.yaml` drops `supabasePersistSessionKey` and registers `defaultPersistSessionKey` under `client.session_management.persist_session`. The capability itself stays `implemented`. The symbol, drift and schema checks were run locally against the base and PR symbol dumps.
…session migration Review of the storage switch turned up four problems. The pkce store changed API and key with no migration, so a magic link or a password reset started on v2 could not be completed after the app updated: the code verifier that began the flow was written through the legacy API and was no longer visible. It is now moved over on the first read that misses. Its constructor also stopped calling WidgetsFlutterBinding.ensureInitialized(), which on v2 ran on every Supabase.initialize and was the only thing initializing the binding when persistSession was false or a custom LocalStorage was passed. Restored. The session migration wrote the new entry only after deleting the legacy one, so an interruption in between lost the session, and it left the legacy cache as a pre-migration snapshot, so on Windows and Linux the app's next legacy write dropped the session that had just been migrated. The new entry is now written first and the legacy store is reloaded before the delete. A failure to read the legacy store took Supabase.initialize with it, leaving the app unable to start over a session it may not even have had. It is logged instead. Also batches the two reads the migration starts with, drops a redundant full reload of the legacy store, documents the widget-test setup the new storage needs, and counts the storage test keys instead of timestamping them, which was flaky on web where the resolution is milliseconds.
Vinzent03
approved these changes
Aug 11, 2026
spydon
enabled auto-merge (squash)
August 11, 2026 16:06
User.fromJson requires created_at since timestamps became DateTime, and the mock response only carried an id, so every test in the file threw.
…preferences-async
mandarini
approved these changes
Aug 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What kind of change does this PR introduce?
Bug fix, breaking change.
What is the current behavior?
SharedPreferencesLocalStorageandSharedPreferencesGotrueAsyncStorage, the storage implementationsSupabase.initializeuses by default, write through the legacySharedPreferencesAPI. On Windows and Linux both APIs rewrite the sameshared_preferences.jsonin full from their own in-memory cache, so an app that has migrated its own code toSharedPreferencesAsynchas its session dropped and the user is signed out on the next launch.Closes #1276, and supersedes #1164.
What is the new behavior?
Both implementations use
SharedPreferencesAsync. The minimum Flutter version is already well past the 3.22 that API needs, so nothing had to be bumped.SharedPreferencesLocalStorage.initialize()moves a session written by v2 over to the new store, so existing users stay signed in:<persistSessionKey>-legacy-migratedkey in the new store records that it happened, so the legacy store is read at most once and a signed-out user is never signed back in.reload()ed first, since the instance is shared with the app and may have been loaded before the session was last written.On web nothing changes: the session still goes into
window.localStorageunder the same key, as it did before.SharedPreferencesGotrueAsyncStorageno longer needs its initializationCompleter, sinceSharedPreferencesAsynchas no asynchronous setup. It is created on first use so that constructing the storage does not require the bindings to be initialized yet.The pending PKCE code verifier is not migrated. It is valid for the length of one sign-in round trip, and the failure mode of losing it is a retryable sign-in.
Superseding #1164
That PR made the same core change. Here is where each point from its review thread landed:
SharedPreferencesAsyncSharedPreferencesAsyncmain: Flutter 3.35, Dart 3.9,shared_preferences: ^2.5.5useSharedPreferencesAsyncflagMIGRATION.mdnow states per platform what mixing costs, including that a session write can drop the app's own legacy preferences, and ships aLocalStorageto copy for staying on the legacy storeSharedPreferencesGotrueAsyncStorageleft on the legacy APIOne comment on that thread reported
refresh_token_already_usedon a cold start after token expiry. That is an auth-refresh problem rather than a storage one, so it is deliberately out of scope here.Additional context
MIGRATION.mddocuments the change, including aLocalStorageimplementation to copy for anyone who wants to keep the session in the legacy store for now.Tests cover the migration: a legacy session is moved over, the legacy entry is gone afterwards, an existing session in the new store wins, a session that was signed out of is not restored on the next launch, the same holds when both stores had a session, and a legacy entry written back after the migration is ignored. They live in a non-web test file, since on web there is nothing to migrate. The other test files now mock both shared_preferences APIs through a
mockSharedPreferenceshelper.Summary by CodeRabbit
localStoragebehavior on web.