fix: decoder and encoder default instances #711
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the default JSONDecoder and JSONEncoder instances to use a centralized supabase configuration and unifies date formatting across the codebase. Key changes include:
- Replacing custom JSON decoding/encoding strategies in various modules with JSONDecoder.supabase() and JSONEncoder.supabase().
- Refactoring date formatting logic in SupabaseLogger and DateFormatter to use the new iso8601String property.
- Updating related tests and configurations (e.g., DotEnv) to ensure consistency.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/IntegrationTests/DotEnv.swift | Added a DotEnv enum with environment configuration keys. |
| Sources/Storage/Codable.swift | Updated defaultStorageDecoder to use the supabase configuration. |
| Sources/PostgREST/Defaults.swift | Refactored jsonDecoder and jsonEncoder to use supabase implementations. |
| Sources/Helpers/SupabaseLogger.swift | Revised date formatting by using the iso8601String property. |
| Sources/Helpers/DateFormatter.swift | Reworked date formatter extensions with new DateFormatter helpers. |
| Sources/Helpers/Codable.swift | Switched to supabase() functions for JSONDecoder and JSONEncoder. |
| Sources/Helpers/AnyJSON/AnyJSON+Codable.swift | Updated AnyJSON encoder/decoder to use supabase implementations. |
| Sources/Auth/Defaults.swift | Adjusted AuthClient default encoders/decoders to utilize supabase(). |
grdsdev
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the default encoder and decoder instances and updates the session storage migration logic. Key changes include:
- Adopting JSONDecoder.supabase() and JSONEncoder.supabase() as default implementations.
- Replacing XCTAssertEqual assertions with expectNoDifference in tests.
- Adding a new storage migration to use the default encoder for sessions.
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/IntegrationTests/DotEnv.swift | Adds DotEnv constants for integration testing. |
| Tests/AuthTests/SessionManagerTests.swift | Updates session refresh assertions using expectNoDifference. |
| Tests/AuthTests/AuthClientTests.swift | Replaces multiple XCTAssertEqual calls with expectNoDifference. |
| Sources/Storage/Codable.swift | Uses JSONDecoder.supabase() for the default storage decoder. |
| Sources/PostgREST/Defaults.swift | Switches JSONDecoder/Encoder to use supabase() implementations. |
| Sources/Helpers/SupabaseLogger.swift | Uses Date.iso8601String for log message formatting. |
| Sources/Helpers/DateFormatter.swift | Refactors date formatting to use DateFormatter-based helpers. |
| Sources/Helpers/Codable.swift | Refactors JSONDecoder/Encoder to expose supabase() functions. |
| Sources/Helpers/AnyJSON/AnyJSON+Codable.swift | Updates AnyJSON to use the supabase defaults. |
| Sources/Auth/Internal/SessionStorage.swift | Updates session storage migration and decoding/encoding methods. |
| Sources/Auth/Defaults.swift | Uses the supabase default encoder/decoder for AuthClient configuration. |
Files not reviewed (1)
- Tests/AuthTests/Resources/local-storage.json: Language not supported
| } | ||
| } | ||
|
|
||
| do { | ||
| let storedData = try storage.retrieve(key: key) | ||
| return try storedData.flatMap { | ||
| try AuthClient.Configuration.jsonDecoder.decode(Session.self, from: $0) | ||
| try JSONDecoder().decode(Session.self, from: $0) |
There was a problem hiding this comment.
Consider using JSONDecoder.supabase() here instead of instantiating a new JSONDecoder(), to ensure consistent decoding behavior across the codebase.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Since this is the decoder used for decoding the stored session, it is safer to use a new decoder instance, unrelated to the other ones used in codebase, since any change to decoder configuration may break the session decoding.
| @@ -64,7 +67,7 @@ extension SessionStorage { | |||
| do { | |||
| try storage.store( | |||
| key: key, | |||
| value: AuthClient.Configuration.jsonEncoder.encode(session) | |||
| value: JSONEncoder().encode(session) | |||
There was a problem hiding this comment.
Consider using JSONEncoder.supabase() here instead of a new JSONEncoder(), to maintain consistent encoding behavior in session storage.
| value: JSONEncoder().encode(session) | |
| value: JSONEncoder.supabase().encode(session) |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Same as the comment related to JSONDecoder.
Pull Request Test Coverage Report for Build 14623634293Details
💛 - Coveralls |
What kind of change does this PR introduce?
Close #702
Introduce a
supabasedefaultJSONEncoderandJSONDecoderfor usage across all sub-packages, this makes JSON decoding configuration consistent across the codebase.Since now all sub-packages uses the same Encoder/Decoder, it fixes an issue that decoding a type in Postgrest works, but not in Realtime change, as reported in #702