supabase / gotrue-dart

A dart client library for GoTrue.
MIT License
46 stars 37 forks source link

Breaking: rework to make it parity with gotrue-js #90

Closed dshukertjr closed 2 years ago

dshukertjr commented 2 years ago

What kind of change does this PR introduce?

Make this library parity with gotrue-js v2.0 rc.

The biggest difference is that with this PR, we have two namespaces:client and client.admin.

client is used for any properties/ methods used within the client side where as any methods in client.admin is meant to be used on the server side.

What is the current behavior?

Some of the methods don't match what is in gotrue-js v2

What is the new behavior?

The list of methods match. There are few missing methods on the GotrueAdmin class, but those are meant to be server side methods, so we can add them later on.

Additional context

So sorry for this long PR. I should have split this one into smaller ones. There are a lot of moving parts in this PR, especially in the public API, so any suggestions/ comments are welcome!

Also, I haven't been able to bring over all the test cases from gotrue-js, so if anyone wants to add more tests to this one, feel free to add onto this PR!

Open id connect related methods have been removed both from this library and the js library, but they will be added back as soon as we sort out the server side issues.

Also, js library uses client.user() and client.session() to get user and session, but I have this library with client.currentUser and client.currentSession. The reason for this is:

,but let me know if we should stick with this, or should we go back to .user() and .session().

dshukertjr commented 2 years ago

Also, now that supabase-js v2.0 is officially released, I'd like to release a stable v1.0 of supabase-flutter once this change is merged, but is there anything else that we have left to do while it is still in developer preview?

Vinzent03 commented 2 years ago

Shouldnt runtimeType be included in the hashcode from AuthException? Two different classes with the same properties would have the same hashcode.

dshukertjr commented 2 years ago

@Vinzent03 I think that part is fine. Quote from Flutter docs:

Objects that are not equal are allowed to have the same hash code. It is even technically allowed that all instances have the same hash code, but if clashes happen too often, it may reduce the efficiency of hash-based data structures like HashSet or HashMap.

dshukertjr commented 2 years ago

@supabase-community/dart-maintainers I was wondering if there are more things that should be changed/ fixed on this one, but are there any?

dshukertjr commented 2 years ago

@Vinzent03 Changing onAuthStateChange into Stream sounds great. Let's see what I can whip up.

dshukertjr commented 2 years ago

With this update, onAuthStateChange is now a Stream and can be used like this. Not sure if AuthState is the best name for it, so am open for suggestions if there are any!

final StreamSubscription<AuthState> subscription = supabase.auth.onAuthStateChange.listen((event) {
  final AuthChangeEvent event = event.event;
  final Session? session = event.session;
  // Do something with event and session
});
Vinzent03 commented 2 years ago

I think the name is fine. I thought about AuthEvent, but that's already somewhat used for the event type (AuthEvent.event). Despite that (hopefully) last comment: LGTM!