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

Add needed base calls when using FluxorComponent #82

Closed
serge-wq opened this issue Aug 19, 2020 · 1 comment · Fixed by #86
Closed

Add needed base calls when using FluxorComponent #82

serge-wq opened this issue Aug 19, 2020 · 1 comment · Fixed by #86

Comments

@serge-wq
Copy link

I was having some issues when disposing a Component that inherited from FluxorComponent until I realized I didn't call base.OnInitialized when overriding the method. This caused the subscription that is disposed later to be null triggering the exception. I'm creating this issue just to suggest that maybe it would be a good idea to add the remark that a call to base.OnInitialized is needed when using FluxorComponent and overriding OnInitialized.

Great work with the library.

@mrpmorris
Copy link
Owner

Yes, that seems like a good idea. If you'd like to submit a PR, I'd be happy to look at it.

if (whatever == null)
  throw new NullReferenceException($"{nameof(whatever)} is null. Either the constructor failed, or an OnInitialized method has been overridden that does not call the base method");

@mrpmorris mrpmorris linked a pull request Aug 25, 2020 that will close this issue
@mrpmorris mrpmorris mentioned this issue Aug 26, 2020
mrpmorris added a commit that referenced this issue Aug 26, 2020
* Allow observer callback code to unsubscribe from action notifications (Fixes #84)

* Throw exception if base.OnInitialzed not called (Fixes #82)

* Remove unneeded reflection (Fixes #87) (#90)

* Include ActionSubscriber tutorial in solution
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 a pull request may close this issue.

2 participants