refactor!: singularize plural enum names and fold enum extensions in - #1654
Conversation
A Dart enum type names one value rather than the set, so its name should be singular. Five enums were ports of realtime-js / gotrue-js names instead: - `ChannelStates` -> `ChannelState` - `ChannelEvents` -> `ChannelEvent` - `RealtimeListenTypes` -> `RealtimeListenType` - `PostgresTypes` -> `PostgresType` - `AuthenticatorAssuranceLevels` -> `AuthenticatorAssuranceLevel` The first three were already unreachable for consumers (omitted from the `show` list or named in the `hide` clause of the realtime_client exports), so they are now also annotated `@internal`, which is both accurate and keeps them out of the capability matrix scan. While sweeping the remaining enums, `LoadTableSnapshots` was the only other name that did not read as a noun naming one value; it is now `TableSnapshotScope`. Enums can declare methods and static methods directly, so every extension that only existed to hang helpers off an enum is folded into the enum itself: - `ChannelEventsExtended` -> `ChannelEvent.fromType` / `ChannelEvent.eventName` - `ToType` -> `RealtimeListenType.toType` - `PostgresChangeEventMethods` -> `PostgresChangeEvent.fromString` / `toRealtimeEvent` - `PresenceEventExtended` -> `PresenceEvent.fromString` - `AuthChangeEventExtended` -> `AuthChangeEvent.fromString` - `GenerateLinkTypeExtended` -> `GenerateLinkType.fromString` Those helpers decode wire values, so the folded members are annotated `@internal`, matching the visibility the extensions had via the `hide` clauses. The now-empty `hide` entries are dropped from the gotrue and realtime_client exports. `SupabaseEventTypesName` is left alone: its member is `name()`, which would shadow `Enum.name` if declared on the enum, and both it and its deprecated enum are due for removal anyway. No enum values change and no behaviour changes: `ChannelEvent.eventName()` still produces the same `phx_*` wire strings, and `TableSnapshotScope` still sends `all` / `refs`. `SocketStates` is left alone here because #1404 already renames it. BREAKING CHANGE: `PostgresTypes` is now `PostgresType`, `AuthenticatorAssuranceLevels` is now `AuthenticatorAssuranceLevel` and `LoadTableSnapshots` is now `TableSnapshotScope`. The `PresenceEventExtended` extension is gone; `PresenceEvent.fromString` replaces it and is now internal. Closes #1652
|
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)
📝 WalkthroughWalkthroughThe PR singularizes enum names across Gotrue, Realtime, and Storage. It moves conversion helpers into enum declarations, marks internal enums, updates call sites and tests, and synchronizes SDK compliance symbols. ChangesEnum API updates
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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
🤖 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/gotrue/lib/src/types/mfa.dart`:
- Line 366: Wrap the documentation comment for the AAL-level declaration near
the visible “Next possible AAL level” text so every Dart source line is no
longer than 80 characters, preserving the existing wording, then run dart format
on the file.
In `@packages/realtime_client/lib/src/types.dart`:
- Around line 69-84: Wrap the long Dart documentation comments in ChannelFilter,
especially the event description and postgresChanges filter description, so
every source line stays within 80 characters. Preserve the existing documented
filter contract and run dart format without changing the surrounding fields or
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: 8868351c-b90c-4095-9baa-d53edeb05a5c
📒 Files selected for processing (21)
packages/gotrue/lib/gotrue.dartpackages/gotrue/lib/src/constants.dartpackages/gotrue/lib/src/gotrue_client.dartpackages/gotrue/lib/src/gotrue_mfa_api.dartpackages/gotrue/lib/src/types/auth_response.dartpackages/gotrue/lib/src/types/mfa.dartpackages/gotrue/test/src/constants_test.dartpackages/gotrue/test/src/gotrue_mfa_api_test.dartpackages/realtime_client/lib/realtime_client.dartpackages/realtime_client/lib/src/constants.dartpackages/realtime_client/lib/src/message.dartpackages/realtime_client/lib/src/push.dartpackages/realtime_client/lib/src/realtime_channel.dartpackages/realtime_client/lib/src/realtime_client.dartpackages/realtime_client/lib/src/transformers.dartpackages/realtime_client/lib/src/types.dartpackages/realtime_client/test/channel_test.dartpackages/realtime_client/test/message_test.dartpackages/realtime_client/test/socket_test.dartpackages/storage_client/lib/src/iceberg/iceberg_types.dartsdk-compliance.yaml
CodeRabbit flagged the AAL and ChannelFilter doc comments in the renamed declarations for running past 80 columns. Wrap them with the wording unchanged, and promote the two `AuthenticatorAssuranceLevel` value comments from `//` to `///` so they actually document the values.
…olations (#1655) ## Why `dart format` never reflows comments and never splits string literals, so nothing enforced the 80-column limit outside of the code the formatter can break by itself. 440 over-long lines had accumulated across 109 files, and CodeRabbit was doing the enforcement by hand in review (it flagged two such lines on #1654). Enabling the rule continues the ratcheting pattern `packages/supabase_lints/lib/analysis_options.yaml` already documents: turn a rule on once its violations are gone. ## What `lines_longer_than_80_chars: true` in the shared lint config (the Flutter variant includes it, so both are covered), plus the 440 fixes. | Category | Count | Treatment | | --- | --- | --- | | Prose in `///` / `//` blocks | 281 + 59 | Rewrapped at 80 columns, wording unchanged | | Over-long string literals | ~100 | Split into adjacent literals (same value) | | Doc code samples | 12 | Reformatted the way `dart format` would break the same code | | Missing space after `///` / `//` | 4 | Added, then wrapped | | Other | 4 | See below | The four one-offs: - `buildClientInfoHeader` computes the encoded platform version into a local instead of interpolating a call chain that cannot fit on one line. Same output. - Two `test(...)` headers with a trailing closure moved their arguments onto separate lines. - One trailing `// comment` on a map entry moved above the entry. Three doc samples were JavaScript left over from the ports (`new RetryTimer(...)`, `console.log(...)`, `let`) inside ` ```dart ` fences. They are Dart now. Fenced code, markdown lists and tables, and `{@template}` macros were kept structurally intact rather than reflowed as prose. ## String values are provably unchanged Splitting a literal into adjacent literals is value-preserving only if the split points are the sole change, so that was checked mechanically rather than by eye: every string literal value in every touched file was parsed with `package:analyzer` before and after (adjacent literals collapsed the way the language does) and compared in order. The only file that reports a difference is `client_info.dart`, which is the deliberate refactor above. ## A note on the rule It tolerates a line whose overflow is a single unbreakable URI, verified with a probe file: a long `'http://…'` literal is accepted, while an equally long literal without a URI is not. So URLs in comments and strings stay as they are, and no `// ignore:` comments were needed anywhere. ## Testing - `dart analyze packages examples`: clean, zero violations - `dart format --set-exit-if-changed packages examples`: clean, so the wrapping is stable under the formatter - Passing suites: `supabase_common`, `functions_client`, `realtime_client`, `storage_client`, `supabase`, `supabase_flutter`, and the hermetic `gotrue` suites (including `get_claims_test` and `jwk_test`, whose JWT fixtures were split most aggressively, and the two restructured tests) - `postgrest` fails against my local stack both on this branch and on its base, and two identical runs of the same code disagree on which tests fail, so that suite is state-dependent locally. CI, which starts a fresh stack, is the arbiter there. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Improved API, security, usage, and example documentation across authentication, database, realtime, storage, and client libraries. * Clarified server-only authentication administration guidance and sign-out event handling. * Reformatted lengthy comments, examples, messages, and test descriptions for improved readability and consistency. * Documented that each JSON isolate instance maintains one running isolate. * **Bug Fixes** * Improved platform-version metadata encoding while preserving spaces. * **Chores** * Enabled enforcement of an 80-character line-length limit. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Closes SDK-1425 ## What v3 is accumulating breaking changes across every package in the monorepo (#1278), and there is nowhere a user can look to find out what they have to change in their own code. The changelogs are per package, auto-generated and one line per commit, so anyone upgrading has to read seven of them and reconstruct the migration themselves. `sdk-compliance.yaml` tracks capabilities, not migrations. This adds a `MIGRATION.md` at the repository root, modelled on [Flame's migration guide](https://github.com/flame-engine/flame/blob/main/doc/flame/migration.md): - One section per major version step, newest first: `## Migrating from v2 to v3`. - One `###` subsection per breaking change, titled after what changed. - Each subsection says why the change was made and what you have to do, with a `// Before` / `// After` Dart snippet. - Changes that keep their name but change type or behaviour are called out explicitly, since those do not surface as compile errors. One file for the whole monorepo rather than one per package, because the packages go major in lockstep, and it gives us a single link to hand to users. ## Covered so far The two breaking commits that have landed on `main`: - `RealtimeClient.connectionState` changing from `String` to `SocketState?` (#1404). This is the silent one: the name is unchanged, so anyone comparing it against `'open'` gets no compile error. - The `conn` abbreviations spelled out: `conn` → `connection`, `connState` → `connectionState`, `onConnMessage` → `onConnectionMessage` (#1404). - The four consumer-reachable enum renames (#1654): `SocketStates` → `SocketState`, `PostgresTypes` → `PostgresType`, `AuthenticatorAssuranceLevels` → `AuthenticatorAssuranceLevel`, `LoadTableSnapshots` → `TableSnapshotScope`. Left out on purpose: the three enums that were never exported (`ChannelStates`, `ChannelEvents`, `RealtimeListenTypes`) and the enum extensions folded in by #1654, since all of them are `@internal` and cannot break a consumer. The section carries a note that v3 is unreleased and the list is still growing. ## Keeping it current `AGENTS.md` now states that a change breaking the public API adds its own section to `MIGRATION.md` in the same pull request, the same way parity work reconciles `sdk-compliance.yaml`. There is no pull request template in `.github/`, so there was nothing to add a checkbox to. Linked from the root `README.md` and `packages/supabase_flutter/README.md`, next to the existing Guides and Reference Docs links. ## Testing Documentation only, no code changes. Every snippet in the guide was type-checked against the workspace in a scratch file (`dart analyze`, clean), which was then deleted.
Closes #1652
What
A Dart enum type names one value rather than the set, so its name should be singular. Five enums were ports of
realtime-js/gotrue-jsnames instead:ChannelStatesChannelStateshowlist)ChannelEventsChannelEventshowlist)RealtimeListenTypesRealtimeListenTypehideclause)PostgresTypesPostgresTypeAuthenticatorAssuranceLevelsAuthenticatorAssuranceLevelThe three unreachable ones are now annotated
@internal, which is both the accurate annotation and enough to keep them out of the capability matrix scan.Sweeping the rest of the monorepo's 51 enums turned up one more name that did not read as a noun naming a single value:
LoadTableSnapshots(storage, Iceberg) is nowTableSnapshotScope.Enum extensions folded in
Enums can declare methods and static methods directly, so every extension that only existed to hang helpers off an enum is now part of the enum:
ChannelEventsExtended→ChannelEvent.fromType/ChannelEvent.eventNameToType→RealtimeListenType.toTypePostgresChangeEventMethods→PostgresChangeEvent.fromString/PostgresChangeEvent.toRealtimeEventPresenceEventExtended→PresenceEvent.fromStringAuthChangeEventExtended→AuthChangeEvent.fromStringGenerateLinkTypeExtended→GenerateLinkType.fromStringThose helpers decode wire values, so the folded members carry
@internal, matching the visibility the extensions already had through thehideclauses. The now-unnecessaryhideentries are dropped from the gotrue and realtime_client exports.SupabaseEventTypesNameis deliberately left alone: its member isname(), which would shadowEnum.nameif declared on the enum, and both it and its deprecated enum are due for removal anyway.Not changed
SocketStates, because refactor(realtime)!: clean up connection naming on RealtimeClient #1404 already renames it.AMRMethod/AMREntryviolate Dart's "capitalize acronyms longer than two letters like words" rule (AmrMethod/AmrEntry). Out of scope here, worth a follow-up.RequestMethodTypein gotrue is replaced by the sharedHttpMethodin refactor!: share one HttpMethod enum across the client packages #1645.FactorStatus,RealtimeHeartbeatStatusandRealtimeSubscribeStatusend insbut are singular ("status").GenerateLinkType.magiclinkandOtpType.magiclinkare not lowerCamelCase, but they are load-bearing: the wire value is derived from the value name.Behaviour
None. No enum value changes:
ChannelEvent.eventName()still produces the samephx_*strings andTableSnapshotScopestill sendsall/refs.Capability matrix
PostgresType,AuthenticatorAssuranceLevelandTableSnapshotScopeare registered underrealtime.subscriptions.postgres_changes,auth.mfa.get_authenticator_assurance_levelandstorage.analytics.iceberg_table(replacing the staleLoadTableSnapshotsentries).Testing
dart analyze packages examples: cleanrealtime_client,supabase,supabase_fluttersuites: passgotrueMFA, constants and types suites: pass (the rest of thegotruesuite has pre-existing failures against my local stack onmaintoo)check-api-symbols,check-driftandvalidate-compliancerun locally against this branch: all passSummary by CodeRabbit
Breaking Changes
Improvements
Tests