Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: expose Realtime options on SupabaseClient #377

Merged
merged 9 commits into from
May 13, 2024

Conversation

bryandubno
Copy link
Contributor

What kind of change does this PR introduce?

Exposes public Realtime configuration options.

What is the current behavior?

Realtime configuration options are public, but cannot be configured by the client.

What is the new behavior?

Realtime configuration options can now be set by the client.

Additional context

This is not ready to be merged. Tests need to be updated.

@bryandubno bryandubno requested a review from grdsdev as a code owner May 10, 2024 17:02
@bryandubno bryandubno marked this pull request as draft May 10, 2024 17:03
@grdsdev
Copy link
Collaborator

grdsdev commented May 13, 2024

Hi @bryandubno this is a nice addition, is there anything I can help to move it forward? What is missing?

@grdsdev grdsdev changed the title Realtime/realtime options feat: expose Realtime options on SupabaseClient May 13, 2024
@bryandubno
Copy link
Contributor Author

bryandubno commented May 13, 2024

Hi @bryandubno this is a nice addition, is there anything I can help to move it forward? What is missing?

Thanks @grdsdev – I just haven't had a chance to run or add any additional unit tests. If you're able to help on that front, I believe this would be ready for review!

I do also wonder if it would be better to add to SupabaseClientOptions instead?

self.connectOnSubscribe = connectOnSubscribe
}
}

public struct Configuration: Sendable {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be better marked as internal given it's not intended to be accessed by consumers

@@ -26,26 +48,22 @@ public actor RealtimeClientV2 {
var disconnectOnSessionLoss: Bool
var connectOnSubscribe: Bool
var logger: (any SupabaseLogger)?

public init(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also might be better marked as internal given it's not intended to be accessed by consumers

@grdsdev
Copy link
Collaborator

grdsdev commented May 13, 2024

@bryandubno

I do also wonder if it would be better to add to SupabaseClientOptions instead?

Yes please, let's add it to SupabaseClientOptions under a realtime attribute.

@grdsdev grdsdev marked this pull request as ready for review May 13, 2024 20:53
@grdsdev
Copy link
Collaborator

grdsdev commented May 13, 2024

@bryandubno I've made a few changes to your code and also updated tests.

@bryandubno
Copy link
Contributor Author

@bryandubno I've made a few changes to your code and also updated tests.

Fantastic! I appreciate you pushing this through!

@grdsdev grdsdev merged commit 9cfafdb into supabase:main May 13, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants