Restructure Project to Support DI and Nullity #36
Conversation
|
@veleek any chance you'd be up to review this one? I appreciated your eyes on realtime-csharp! |
There was a problem hiding this comment.
Nothing seems obviously broken or anything. Got lots of comments on design and other potential updates to make.
I'm always a very picky code reviewer, so feel free to ignore any of my comments and I won't be upset or anything. Just looking to provide as much input as I can.
|
Also wanted to say "Thanks" for this change. I literally JUST started integrating this into my project and was running into problems with the async initialization and dependency injection stuff. Perfect timing! |
|
@veleek Thanks for how thorough your review is! It's very much appreciated. DI is not my forte, so I'm trying to my best with the request for it! I'll need to update the functions client with your suggested change on the URL being set for the whole client as opposed to per method, and that ought to remove the need for the |
- Fix scope on private variables that are only used at initialization.
|
Phew. Alright @veleek - I think that's the last commit. Care to lend me your eyes again? |
Supabase/Client.cs
Outdated
| } | ||
| _postgrestClient.Options.Headers = GetAuthHeaders(); | ||
| _storageClient.Headers = GetAuthHeaders(); | ||
| _postgrest.Options.Headers = GetAuthHeaders(); |
There was a problem hiding this comment.
_postgrest already has GetHeaders set to call GetAuthHeaders() so I don't think this is necessary. It'll implicitly use the updated auth token for you so you can just remove these two lines.
| headers["X-Client-Info"] = Util.GetAssemblyVersion(); | ||
|
|
||
| if (supabaseKey != null) |
There was a problem hiding this comment.
Just spitballing here... would it be crazy to move supabaseKey into the Options object.
|
lol practically all of your suggestions. But to be fair, very good suggestions! As I said before, thanks so much for your help on this! Nice to have some extra eyes on it. |
In reference to: #23, #34, #35.
This pull request implements the major api changes introduced in:
It also introduces some major changes to the Supabase C# library.
Clientis no longer a singleton, singleton interactions (if desired) are left to the developer to implement.Clientsupports injection of dependent clients after initialization via property:AuthClientFunctionsClientRealtimeClientPostgrestClientStorageClientSupabaseModelcontains no logic but remains for backwards compatibility. (Marked Obsolete)ClientOptions.ShouldInitializeRealtimewas removed (no longer auto initialized)ClientOptionsnow references anISupabaseSessionHandlerwhich specifies expected functionality for session persistence on Gotrue (replacesClientOptions.SessionPersistor,ClientOptions.SessionRetriever, andClientOptions.SessionDestroyer).