change subject.unsubscribe() to subject.complete()

[sougiovn]

This pull request is related to a bug I found out when testing the ngx-errors to use on my project. I discovered that another user had already opened an issue #22 .
The bug occurs because the BehaviorSubject is closed when ngOnDestroy is called, but don't recieve another instance when a new component is created, making it impossible to use this component with ngIf directive.
Hunting down this bug I found out that the BehaviorSubject references was kind of "dirty" because it was being unsubscribed() when the ngOnDestroy from ngxerrors was being called.
As the ngxerrors wasn't subscribing to it, there was no reason to unsubscribe(), to leave this reference you should complete the subject as the ngxerrors is it's manager.
By completing it, all subscriptions should be already unsubscribed and the components was ready to receive other instance of BehaviorSubject when a new component is created.
And passed the instantiation of the BehaviorSubject to the ngOnInit lifecycle hook.

What are you adding/fixing?
Bug fix #22

Have you added tests for your changes?
I tried to add, but failed. I couldn't test if the fixture.detectChanges() was triggering and error.

Will this need documentation changes?

No

Does this introduce a breaking change?

No

Other information

[sougiovn]

Don't know why CI tests failed, but running yarn test locally everything passed.

[alignsoft]

I've been noticing these errors when my project restarts in dev after a code change/rebuild, and I'm assuming they're related to this issue.

ObjectUnsubscribedError: object unsubscribed
    at new ObjectUnsubscribedError (http://localhost:4000/vendor/vendor.js:95600:26)
    at BehaviorSubject.__vendor../node_modules/rxjs/Subject.js.Subject.next (http://localhost:4000/vendor/vendor.js:76846:19)
    at BehaviorSubject.__vendor../node_modules/rxjs/BehaviorSubject.js.BehaviorSubject.next (http://localhost:4000/vendor/vendor.js:75893:31) at 
NgxErrorsDirective../node_modules/@ultimate/ngxerrors/src/ngxerrors.directive.js.NgxErrorsDirective.checkStatus (http://localhost:4000/build/app.js:39410:26)
    at http://localhost:4000/build/app.js:39419:19
    at ZoneDelegate.__vendor../node_modules/zone.js/dist/zone.js.ZoneDelegate.invokeTask (http://localhost:4000/vendor/vendor.js:97014:31)
    at Object.onInvokeTask (http://localhost:4000/vendor/vendor.js:36284:33)
    at ZoneDelegate.__vendor../node_modules/zone.js/dist/zone.js.ZoneDelegate.invokeTask (http://localhost:4000/vendor/vendor.js:97013:36)
    at Zone.__vendor../node_modules/zone.js/dist/zone.js.Zone.runTask (http://localhost:4000/vendor/vendor.js:96781:47)
    at __vendor../node_modules/zone.js/dist/zone.js.ZoneTask.invokeTask (http://localhost:4000/vendor/vendor.js:97088:34)"
__proto__:
Error

[jc-white]

Any update on this? I am seeing this error still when using with ngIf. We would love to use it in production soon. I know you're a busy guy @toddmotto - thank you for providing this module.

[sougiovn]

Well, until the PR get approved I published my forked version
@jarinwhite @alignsoft

https://www.npmjs.com/package/ngx-errors

[virgil-av]

Any idea when this will be merged, this error is still present and when combined with *ngIf

[sougiovn]

@virgil-av waiting for @toddmotto

[virgil-av]

Thank you @gigioSouza until @toddmotto approves this PR I will remove ngx-error from the project as I have a few forms that relay on *ngIf to hide and show, but good job on fixing the problem so clean.

[szilarddavid]

@gigioSouza that package doesn't work for me, looks to contain the whole github repo.

I've set up an npm package which contains the changes in this pull request

https://www.npmjs.com/package/ngx-errors-complete

[alignsoft]

I've tried using the forked version of this @szilarddavid posted, and I'm getting the exact same error.

Just so I'm clear - I'm seeing this on components where *ngIf is used to hide a div that contains an ngx-error directive, and the 'TypeError: Cannot read property 'unsubscribe' of undefined' is thrown when the containing component is destroyed.

Is that correct? Should the modified ngx-errors-complete resolve this issue?