Skip to content

Commit

Permalink
fix(ng-core): DirectiveSuperclass.getInput$() emits synchronously i…
Browse files Browse the repository at this point in the history
…f `.ngOnChanges()` was already called
  • Loading branch information
ersimont committed Jan 7, 2021
1 parent 3c2799a commit 466415c
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 16 deletions.
3 changes: 0 additions & 3 deletions TODO.md
@@ -1,6 +1,3 @@
- Investigate error while build `ng-dev`:
> WARNING: No name was provided for external module '@s-libs/js-core' in output.globals – guessing 'jsCore'
> WARNING: No name was provided for external module '@s-libs/micro-dash' in output.globals – guessing 'microDash'
- landing page to link to all API docs
- Set width/height/border for the wrapper component in `ComponentContextNext`
- coveralls
Expand Down
58 changes: 51 additions & 7 deletions projects/ng-core/src/lib/directive-superclass.spec.ts
Expand Up @@ -5,6 +5,7 @@ import {
Injector,
Input,
OnChanges,
OnInit,
Pipe,
PipeTransform,
SimpleChanges,
Expand Down Expand Up @@ -196,7 +197,7 @@ describe('DirectiveSuperclass', () => {
});

// https://github.com/simontonsoftware/s-libs/issues/14
it('emits immediately (only) if `ngOnChanges()` was already called', () => {
it('does not emit until ngOnChanges is called', () => {
@Component({ template: '' })
class TestDirective extends DirectiveSuperclass implements OnChanges {
@Input() myInput?: string;
Expand Down Expand Up @@ -239,19 +240,62 @@ describe('DirectiveSuperclass', () => {
}
}

@Component({ template: '<s-no-input></s-no-input>' })
class WrapperComponent {}

const ctx2 = new ComponentContextNext(WrapperComponent, {
declarations: [NoInputComponent],
});
const ctx2 = new ComponentContextNext(
NoInputComponent,
{ declarations: [NoInputComponent] },
['myInput'],
);
ctx2.run(() => {
const testDirective = ctx2.fixture.debugElement
.query(By.directive(NoInputComponent))
.injector.get(NoInputComponent);
expect(testDirective.emitted).toBe(true);
});
});

it('emits immediately if ngOnChanges was already called (prerelease bug)', () => {
@Component({ template: `{{ boundValue.name }}` })
class InputBindingComponent
extends DirectiveSuperclass
implements OnInit {
@Input() inputValue!: { name: string };
boundValue!: { name: string };

constructor(injector: Injector) {
super(injector);
this.getInput$('inputValue').subscribe();
}

ngOnInit(): void {
this.bindToInstance('boundValue', this.getInput$('inputValue'));
}
}

const ctx2 = new ComponentContextNext(InputBindingComponent);
ctx2.assignInputs({ inputValue: { name: 'Techgeek19' } });
ctx2.run(() => {
expect(ctx2.fixture.nativeElement.textContent).toContain('Techgeek19');
});
});

it('emits immediately during the first call to ngOnChanges (prerelease bug) 2', () => {
@Component({ template: `{{ boundValue.name }}` })
class InputBindingComponent extends DirectiveSuperclass {
@Input() inputValue!: { name: string };
boundValue!: { name: string };

constructor(injector: Injector) {
super(injector);
this.bindToInstance('boundValue', this.getInput$('inputValue'));
}
}

const ctx2 = new ComponentContextNext(InputBindingComponent);
ctx2.assignInputs({ inputValue: { name: 'Techgeek19' } });
ctx2.run(() => {
expect(ctx2.fixture.nativeElement.textContent).toContain('Techgeek19');
});
});
});

describe('.bindToInstance()', () => {
Expand Down
23 changes: 17 additions & 6 deletions projects/ng-core/src/lib/directive-superclass.ts
Expand Up @@ -5,9 +5,10 @@ import {
OnChanges,
SimpleChanges,
} from '@angular/core';
import { isTruthy } from '@s-libs/js-core';
import { delayOnMicrotaskQueue } from '@s-libs/rxjs-core';
import { Observable, Subject } from 'rxjs';
import { filter, map, startWith } from 'rxjs/operators';
import { BehaviorSubject, merge, Observable, of, Subject } from 'rxjs';
import { distinctUntilChanged, filter, map } from 'rxjs/operators';
import { InjectableSuperclass } from './injectable-superclass';

/**
Expand Down Expand Up @@ -60,6 +61,8 @@ export abstract class DirectiveSuperclass

protected changeDetectorRef: ChangeDetectorRef;

#onChangesRan$ = new BehaviorSubject(false);

constructor(injector: Injector) {
super();
this.changeDetectorRef = injector.get(ChangeDetectorRef);
Expand All @@ -69,17 +72,25 @@ export abstract class DirectiveSuperclass
this.inputChanges$.next(
new Set(Object.getOwnPropertyNames(changes) as Array<keyof this>),
);
this.#onChangesRan$.next(true);
}

/**
* @return an observable of the values for one of this directive's `@Input()` properties
*/
getInput$<K extends keyof this>(key: K): Observable<this[K]> {
return this.inputChanges$.pipe(
filter((keys) => keys.has(key)),
startWith(undefined),
delayOnMicrotaskQueue(),
// Should emit:
// - immediately if ngOnChanges was already called
// - on the first call to ngOnChanges
// - after a delay if ngOnChanges is never called (when nothing is bound to the directive)
// - when the value actually changes
return merge(
this.#onChangesRan$.pipe(filter(isTruthy), distinctUntilChanged()),
this.inputChanges$,
of(0).pipe(delayOnMicrotaskQueue()),
).pipe(
map(() => this[key]),
distinctUntilChanged(),
);
}

Expand Down

0 comments on commit 466415c

Please sign in to comment.