Skip to content

Commit

Permalink
fix(directionality): complete dir change observable (#8874)
Browse files Browse the repository at this point in the history
Since it's most likely to use the `Dir.change` observable programmatically, rather than through an event binding, we should complete it in order to ensure that everything is cleaned up.
  • Loading branch information
crisbeto authored and jelbourn committed Jan 4, 2018
1 parent 58b93dc commit 41f5fe2
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 3 deletions.
12 changes: 9 additions & 3 deletions src/cdk/bidi/dir.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import {
Directive,
Output,
Input,
EventEmitter
EventEmitter,
AfterContentInit,
OnDestroy,
} from '@angular/core';

import {Direction, Directionality} from './directionality';
Expand All @@ -27,7 +29,7 @@ import {Direction, Directionality} from './directionality';
host: {'[dir]': 'dir'},
exportAs: 'dir',
})
export class Dir implements Directionality {
export class Dir implements Directionality, AfterContentInit, OnDestroy {
_dir: Direction = 'ltr';

/** Whether the `value` has been set to its initial value. */
Expand All @@ -40,7 +42,7 @@ export class Dir implements Directionality {
@Input('dir')
get dir(): Direction { return this._dir; }
set dir(v: Direction) {
let old = this._dir;
const old = this._dir;
this._dir = v;
if (old !== this._dir && this._isInitialized) {
this.change.emit(this._dir);
Expand All @@ -54,5 +56,9 @@ export class Dir implements Directionality {
ngAfterContentInit() {
this._isInitialized = true;
}

ngOnDestroy() {
this.change.complete();
}
}

13 changes: 13 additions & 0 deletions src/cdk/bidi/directionality.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,19 @@ describe('Directionality', () => {
expect(injectedDirectionality.value).toBe('ltr');
expect(fixture.componentInstance.changeCount).toBe(1);
}));

it('should complete the change stream on destroy', fakeAsync(() => {
const fixture = TestBed.createComponent(ElementWithDir);
const dir =
fixture.debugElement.query(By.directive(InjectsDirectionality)).componentInstance.dir;
const spy = jasmine.createSpy('complete spy');
const subscription = dir.change.subscribe(undefined, undefined, spy);

fixture.destroy();
expect(spy).toHaveBeenCalled();
subscription.unsubscribe();
}));

});
});

Expand Down

0 comments on commit 41f5fe2

Please sign in to comment.