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

ListenerRegistration should implement IDisposable #746

Closed
Thaina opened this issue Aug 5, 2020 · 11 comments
Closed

ListenerRegistration should implement IDisposable #746

Thaina opened this issue Aug 5, 2020 · 11 comments

Comments

@Thaina
Copy link
Contributor

Thaina commented Aug 5, 2020

Please fill in the following fields:

Unity editor version: All
Firebase Unity SDK version: At least 6.15.2 and lower
Source you installed the SDK (.unitypackage or Unity Package Manager): UPM
Firebase plugins in use (Auth, Database, etc.): FireStore

Please describe the issue here:

Some 3rd party made to be general listener (honestly https://github.com/neuecc/UniRx)

I want to use FireStore Listener API with it but then ListenerRegistration need to implement IDisposable for compatible with it

Could you please add it like this

	public sealed class ListenerRegistration : IDisposable
	{
		public Task ListenerTask { get; }

		public void Stop();
		public void Dispose() => listener.Stop();
	}

So I could use UniRX like so

	Observable.Create<QuerySnapshot>((observer) => {
		return colReference.Listen((snap) => observer.OnNext(snap));
	});
@Thaina Thaina added the new New issue. label Aug 5, 2020
@google-oss-bot
Copy link

This issue does not seem to follow the issue template. Make sure you provide all the required information.

@DellaBitta DellaBitta added needs-attention Need Googler's attention type: feature request and removed new New issue. labels Aug 5, 2020
@rafikhan rafikhan self-assigned this Aug 5, 2020
@rafikhan
Copy link

rafikhan commented Aug 5, 2020

Thanks for reporting this. We discussed this internally and plan to work on this in a future release.

b/162954556

@rafikhan rafikhan removed the needs-attention Need Googler's attention label Aug 5, 2020
@rafikhan rafikhan removed their assignment Aug 5, 2020
@Thaina
Copy link
Contributor Author

Thaina commented May 25, 2021

@rafikhan Have this request made any progressed?

@dconeybe
Copy link

Unfortunately, there is no update to share. For now, you can work around this by wrapping the ListenerRegistration object returned from Listen() in a custom object that you create that implements IDisposable and whose Dispose() method calls listener.Stop().

@Thaina
Copy link
Contributor Author

Thaina commented May 25, 2021

@dconeybe That's what I have done so far and the point is I want to remove this pollution from my code

@dconeybe
Copy link

Understood. I apologize for the delay and the obvious annoyance this is causing. I'll update this issue once the fix is ready for release which, unfortunately, I do not have any timeline for at this point.

@Thaina
Copy link
Contributor Author

Thaina commented Jun 16, 2021

@dconeybe This was already more than half a year for just 1 second to attach 2 lines of code. Even it cannot be released it should have been done and guarantee to available in the next version already without the need promising any timeline

@dconeybe
Copy link

@Thaina I totally get your frustration. It is literally a 2-line fix and it seems ridiculous that it would take months to get fixed. The work has been prioritized but, since this bug has a workaround, it has not received attention yet. Unfortunately, I still can't make any statements about which version of the Unity SDK will contain the fix but I will post a comment to this bug once it's available.

@dconeybe
Copy link

Update: The fix for this issue was submitted today by @ehsannas (cl/380662743 for Googlers). It will be included in the next Unity SDK release, planned to be version 8.1.0.

@Thaina
Copy link
Contributor Author

Thaina commented Jun 22, 2021

Thank you very much

@Thaina Thaina closed this as completed Jul 20, 2021
@dconeybe
Copy link

The fix has been included in yesterday's Unity SDK 8.1.0 release.

https://firebase.google.com/support/release-notes/unity#version_810_-_july_19_2021

@firebase firebase locked and limited conversation to collaborators Aug 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants