Moved super.dispose down

[ikbendewilliam]

Fixes #802

[rrousselGit]

Hello!

Could you write tests for these changes?

[ikbendewilliam]

I'm having trouble writing a good test, I can't seem to recreate the same issue as we have in our plugin. I did add a test that validates the delegate isn't disposed before the listeners have been removed though 🤔

[rrousselGit]

@ikbendewilliam Did these changes fix the issue with your plugin?

[rrousselGit]

LGTM, but make sure it works on your app first. It'd be a shame if it didn't

[ikbendewilliam]

@rrousselGit Turns out I'm sick, so when I'm feeling better I'll have another go at writing better tests and I'll confirm that everything is fixed for our plugin. Thank you for your patience

[rrousselGit]

No problem! Thanks for your work :)

[ikbendewilliam]

I have good news and bad news 😅
The good news is that I'm able to reproduce it. The bad news is that I used two providers and the error I was receiving was not because the order of the dispose is wrong, but because dispose got called a second time.
This branch obviously doesn't fix that since the issue is not in the package.

My opinion, however, is still that the order should be first remove listeners and then super.dispose. So for me this can be merged, WDYT?

[rrousselGit]

Honestly I'm a bit skeptical.
This could be breaking some folks, since that behavior has been around for ages now.

And the current behavior isn't problematic. You're the first to point it out, I never saw/heard of the issue before.
Removing the listeners after dispose is generally harmless for most provided objects.

[rrousselGit]

Hello again!

I don't think I will go through with these changes after-all. In the end:

• provider is in Long Term Maintenance mode. I avoid to avoid breaking changes
• this could be silently breaking
• the value is not that large
• people haven't requested for this before.

So I'll keep things as they are for now.
But this could be revisited if a large number of people ask for this

I appreciate your effort though!

[snaeji]

We are in the same situation. It would be very beneficial for us if this would be merged so we don't have to fork 👍