Merge pull request #74 from cloudnc/fix/ensure-ng-on-changes-original-hook-defined-when-using-the-same-observable

fix: ensure the definition of the hook ngOnChanges if the corresponding observable is used. See https://stackoverflow.com/a/77930589/2398593

Author: maxime1992

README.md

@@ -85,15 +85,15 @@ Here's an example component that hooks onto the full set of available hooks.
 ```ts
 // ./src/app/child/child.component.ts
 
-import { ChangeDetectionStrategy, Component, Input } from '@angular/core';
+import { ChangeDetectionStrategy, Component, Input, OnChanges } from '@angular/core';
 import { getObservableLifecycle } from 'ngx-observable-lifecycle';
 
 @Component({
   selector: 'app-child',
   templateUrl: './child.component.html',
   changeDetection: ChangeDetectionStrategy.OnPush,
 })
-export class ChildComponent {
+export class ChildComponent implements OnChanges {
   @Input() input1: number | undefined | null;
   @Input() input2: string | undefined | null;
 
@@ -134,6 +134,11 @@ export class ChildComponent {
       changes.input2?.previousValue; // `string | null | undefined`
     });
   }
+
+  // when using the ngOnChanges hook, you have to define the hook in your class even if it's empty
+  // See https://stackoverflow.com/a/77930589/2398593 for more info
+  // eslint-disable-next-line @angular-eslint/no-empty-lifecycle-method
+  public ngOnChanges() {}
 }
 
 ```

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

@@ -1,4 +1,4 @@
-import { Component, OnDestroy, OnInit } from '@angular/core';
+import { Component, OnChanges, OnDestroy, OnInit } from '@angular/core';
 import { isObservable } from 'rxjs';
 import { getObservableLifecycle } from './ngx-observable-lifecycle';
 
@@ -106,5 +106,46 @@ describe('ngx-observable-lifecycle', () => {
 
       expect(originalOnDestroySpy).toHaveBeenCalled();
     });
+
+    // see https://stackoverflow.com/a/77930589/2398593
+    it(`should throw if ngOnChanges isn't defined in the component if ngOnChanges observable is used`, () => {
+      class LocalTestComponent {
+        constructor() {
+          // all except ngOnChanges
+          // even without having the original `ngOnChanges` hook it should be ok
+          const {
+            ngOnInit,
+            ngDoCheck,
+            ngAfterContentInit,
+            ngAfterContentChecked,
+            ngAfterViewInit,
+            ngAfterViewChecked,
+            ngOnDestroy,
+          } = getObservableLifecycle(this);
+        }
+      }
+
+      expect(() => new LocalTestComponent()).not.toThrowError();
+
+      class LocalTestWithNgOnChangesNoOriginalHookComponent {
+        constructor() {
+          // without having the original `ngOnChanges` hook it should throw
+          const { ngOnChanges } = getObservableLifecycle(this);
+        }
+      }
+      expect(() => new LocalTestWithNgOnChangesNoOriginalHookComponent()).toThrowError(
+        `When using the ngOnChanges hook, you have to define the hook in your class even if it's empty. See https://stackoverflow.com/a/77930589/2398593`,
+      );
+
+      class LocalTestWithNgOnChangesAndOriginalHookComponent implements OnChanges {
+        constructor() {
+          // when we have the original `ngOnChanges` hook it should **not** throw
+          const { ngOnChanges } = getObservableLifecycle(this);
+        }
+
+        public ngOnChanges(): void {}
+      }
+      expect(() => new LocalTestWithNgOnChangesAndOriginalHookComponent()).not.toThrowError();
+    });
   });
 });

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

@@ -67,6 +67,12 @@ type PatchedComponentInstance<Hooks extends LifecycleHookKey = any> = Pick<AllHo
 };
 
 function getSubjectForHook(componentInstance: PatchedComponentInstance, hook: LifecycleHookKey): Subject<void> {
+  if (hook === 'ngOnChanges' && !componentInstance.constructor.prototype[hook]) {
+    throw new Error(
+      `When using the ngOnChanges hook, you have to define the hook in your class even if it's empty. See https://stackoverflow.com/a/77930589/2398593`,
+    );
+  }
+
   if (!componentInstance[hookSubject]) {
     componentInstance[hookSubject] = {};
   }

src/app/child/child.component.ts

@@ -1,12 +1,12 @@
-import { ChangeDetectionStrategy, Component, Input } from '@angular/core';
+import { ChangeDetectionStrategy, Component, Input, OnChanges } from '@angular/core';
 import { getObservableLifecycle } from 'ngx-observable-lifecycle';
 
 @Component({
   selector: 'app-child',
   templateUrl: './child.component.html',
   changeDetection: ChangeDetectionStrategy.OnPush,
 })
-export class ChildComponent {
+export class ChildComponent implements OnChanges {
   @Input() input1: number | undefined | null;
   @Input() input2: string | undefined | null;
 
@@ -47,4 +47,9 @@ export class ChildComponent {
       changes.input2?.previousValue; // `string | null | undefined`
     });
   }
+
+  // when using the ngOnChanges hook, you have to define the hook in your class even if it's empty
+  // See https://stackoverflow.com/a/77930589/2398593 for more info
+  // eslint-disable-next-line @angular-eslint/no-empty-lifecycle-method
+  public ngOnChanges() {}
 }