Skip to content

Commit a5a0135

Browse files
authored
Merge pull request #63 from maxime1992/fix/ngonchanges
fix(Hooks): original ngOnChanges not receiving the SimpleChanges and provide the SimpleChanges to our observable
2 parents 17fe079 + 36eae24 commit a5a0135

File tree

2 files changed

+95
-19
lines changed

2 files changed

+95
-19
lines changed

projects/ngx-observable-lifecycle/src/lib/ngx-observable-lifecycle.integration.spec.ts

Lines changed: 80 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,17 @@ import {
77
ChangeDetectionStrategy,
88
Component,
99
DoCheck,
10+
Input,
1011
OnChanges,
1112
OnDestroy,
1213
OnInit,
14+
SimpleChange,
15+
SimpleChanges,
1316
} from '@angular/core';
1417
import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
1518
import { By } from '@angular/platform-browser';
1619
import { getObservableLifecycle } from 'ngx-observable-lifecycle';
17-
import { mapTo } from 'rxjs/operators';
20+
import { map } from 'rxjs/operators';
1821

1922
describe('integration', () => {
2023
type ObserverSpy = {
@@ -59,6 +62,8 @@ describe('integration', () => {
5962
AfterContentChecked,
6063
AfterContentInit
6164
{
65+
@Input() input: any;
66+
6267
public componentInstanceId = componentInstanceId++;
6368

6469
public ngAfterContentChecked(): void {
@@ -81,8 +86,8 @@ describe('integration', () => {
8186
ngDoCheckSpy();
8287
}
8388

84-
public ngOnChanges(): void {
85-
ngOnChangesSpy();
89+
public ngOnChanges(simpleChanges: SimpleChanges): void {
90+
ngOnChangesSpy(simpleChanges);
8691
}
8792

8893
public ngOnDestroy(): void {
@@ -105,14 +110,16 @@ describe('integration', () => {
105110
ngOnDestroy,
106111
} = getObservableLifecycle(this);
107112

108-
ngOnChanges.pipe(mapTo(this.componentInstanceId)).subscribe(onChanges$Spy);
109-
ngOnInit.pipe(mapTo(this.componentInstanceId)).subscribe(onInit$Spy);
110-
ngDoCheck.pipe(mapTo(this.componentInstanceId)).subscribe(doCheck$Spy);
111-
ngAfterContentInit.pipe(mapTo(this.componentInstanceId)).subscribe(afterContentInit$Spy);
112-
ngAfterContentChecked.pipe(mapTo(this.componentInstanceId)).subscribe(afterContentChecked$Spy);
113-
ngAfterViewInit.pipe(mapTo(this.componentInstanceId)).subscribe(afterViewInit$Spy);
114-
ngAfterViewChecked.pipe(mapTo(this.componentInstanceId)).subscribe(afterViewChecked$Spy);
115-
ngOnDestroy.pipe(mapTo(this.componentInstanceId)).subscribe(onDestroy$Spy);
113+
const instanceId = this.componentInstanceId;
114+
115+
ngOnChanges.pipe(map(value => ({ instanceId, value }))).subscribe(onChanges$Spy);
116+
ngOnInit.pipe(map(() => ({ instanceId }))).subscribe(onInit$Spy);
117+
ngDoCheck.pipe(map(() => ({ instanceId }))).subscribe(doCheck$Spy);
118+
ngAfterContentInit.pipe(map(() => ({ instanceId }))).subscribe(afterContentInit$Spy);
119+
ngAfterContentChecked.pipe(map(() => ({ instanceId }))).subscribe(afterContentChecked$Spy);
120+
ngAfterViewInit.pipe(map(() => ({ instanceId }))).subscribe(afterViewInit$Spy);
121+
ngAfterViewChecked.pipe(map(() => ({ instanceId }))).subscribe(afterViewChecked$Spy);
122+
ngOnDestroy.pipe(map(() => ({ instanceId }))).subscribe(onDestroy$Spy);
116123
}
117124
}
118125

@@ -131,14 +138,37 @@ describe('integration', () => {
131138
}
132139
}
133140

141+
@Component({
142+
selector: 'lib-host-with-input-component',
143+
template: `
144+
<h1>Host with input Component</h1>
145+
<lib-test-component *ngIf="testComponentVisible" [input]="inputValue"></lib-test-component>
146+
`,
147+
})
148+
class HostWithInputComponent {
149+
public testComponentVisible = false;
150+
151+
public inputValue = undefined;
152+
153+
public setTestComponentVisible(visible: boolean) {
154+
this.testComponentVisible = visible;
155+
}
156+
157+
public setInputValue(value: any) {
158+
this.inputValue = value;
159+
}
160+
}
161+
134162
let component: HostComponent;
163+
let componentWithInput: HostWithInputComponent;
135164
let fixture: ComponentFixture<HostComponent>;
165+
let fixtureWithInput: ComponentFixture<HostWithInputComponent>;
136166

137167
beforeEach(
138168
waitForAsync(() => {
139169
TestBed.configureTestingModule({
140170
imports: [CommonModule],
141-
declarations: [HostComponent, TestComponent],
171+
declarations: [HostComponent, HostWithInputComponent, TestComponent],
142172
}).compileComponents();
143173
}),
144174
);
@@ -167,6 +197,10 @@ describe('integration', () => {
167197
fixture = TestBed.createComponent(HostComponent);
168198
component = fixture.componentInstance;
169199
fixture.detectChanges();
200+
201+
fixtureWithInput = TestBed.createComponent(HostWithInputComponent);
202+
componentWithInput = fixtureWithInput.componentInstance;
203+
fixtureWithInput.detectChanges();
170204
});
171205

172206
it('should be created', () => {
@@ -239,10 +273,42 @@ describe('integration', () => {
239273

240274
newInstance.ngOnDestroy();
241275

242-
expect(onDestroy$Spy.next).toHaveBeenCalledWith(newInstance.componentInstanceId);
276+
expect(onDestroy$Spy.next).toHaveBeenCalledWith({ instanceId: newInstance.componentInstanceId });
243277

244278
const componentUnderTest = fixture.debugElement.query(By.directive(TestComponent)).componentInstance;
245279

246-
expect(onDestroy$Spy.next).not.toHaveBeenCalledWith(componentUnderTest.componentInstanceId);
280+
expect(onDestroy$Spy.next).not.toHaveBeenCalledWith({ instanceId: componentUnderTest.componentInstanceId });
281+
});
282+
283+
it('should still receive the SimpleChanges object in the ngOnChanges original hook and provide the SimpleChanges into the stream as well', () => {
284+
expect(onChanges$Spy.next).not.toHaveBeenCalled();
285+
expect(ngOnChangesSpy).not.toHaveBeenCalled();
286+
componentWithInput.setTestComponentVisible(true);
287+
fixtureWithInput.detectChanges();
288+
289+
expect(onChanges$Spy.next).toHaveBeenCalledOnceWith({
290+
instanceId: jasmine.anything(),
291+
value: {
292+
input: new SimpleChange(undefined, undefined, true),
293+
},
294+
});
295+
expect(ngOnChangesSpy).toHaveBeenCalledOnceWith({
296+
input: new SimpleChange(undefined, undefined, true),
297+
});
298+
299+
componentWithInput.setInputValue('New value');
300+
fixtureWithInput.detectChanges();
301+
302+
expect(onChanges$Spy.next).toHaveBeenCalledTimes(2);
303+
expect(onChanges$Spy.next).toHaveBeenCalledWith({
304+
instanceId: jasmine.anything(),
305+
value: {
306+
input: new SimpleChange(undefined, 'New value', false),
307+
},
308+
});
309+
expect(ngOnChangesSpy).toHaveBeenCalledTimes(2);
310+
expect(ngOnChangesSpy).toHaveBeenCalledWith({
311+
input: new SimpleChange(undefined, 'New value', false),
312+
});
247313
});
248314
});

projects/ngx-observable-lifecycle/src/lib/ngx-observable-lifecycle.ts

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,13 @@ export type LifecycleHookKey = keyof AllHooks;
2626
type AllHookOptions = Record<LifecycleHookKey, true>;
2727
type DecorateHookOptions = Partial<AllHookOptions>;
2828

29-
export type DecoratedHooks = Record<LifecycleHookKey, Observable<void>>;
30-
export type DecoratedHooksSub = Record<LifecycleHookKey, Subject<void>>;
29+
// none of the hooks have arguments, EXCEPT ngOnChanges which we need to handle differently
30+
export type DecoratedHooks = Record<Exclude<LifecycleHookKey, 'ngOnChanges'>, Observable<void>> & {
31+
ngOnChanges: Observable<Parameters<OnChanges['ngOnChanges']>[0]>;
32+
};
33+
export type DecoratedHooksSub = {
34+
[k in keyof DecoratedHooks]: DecoratedHooks[k] extends Observable<infer U> ? Subject<U> : never;
35+
};
3136

3237
type PatchedComponentInstance<K extends LifecycleHookKey> = Pick<AllHooks, K> & {
3338
[hookSubject]: Pick<DecoratedHooksSub, K>;
@@ -55,9 +60,14 @@ function getSubjectForHook(componentInstance: PatchedComponentInstance<any>, hoo
5560
if (!proto[hooksPatched][hook]) {
5661
const originalHook = proto[hook];
5762

58-
proto[hook] = function (this: PatchedComponentInstance<typeof hook>) {
59-
(originalHook as () => void)?.call(this);
60-
this[hookSubject]?.[hook]?.next();
63+
proto[hook] = function (...args: any[]) {
64+
originalHook?.call(this, ...args);
65+
66+
if (hook === 'ngOnChanges') {
67+
this[hookSubject]?.[hook]?.next(args[0]);
68+
} else {
69+
this[hookSubject]?.[hook]?.next();
70+
}
6171
};
6272

6373
const originalOnDestroy = proto.ngOnDestroy;

0 commit comments

Comments
 (0)