fix: invoke changeChanges once on input change (#339)

Author: timdeschryver

projects/testing-library/src/lib/testing-library.ts

@@ -126,15 +126,9 @@ export async function render<SutType, WrapperType = SutType>(
       return;
     }
 
-    const changes = getChangesObj(fixture.componentInstance as Record<string, any>, changedInputProperties);
-
     setComponentInputs(fixture, changedInputProperties);
 
-    if (hasOnChangesHook(fixture.componentInstance)) {
-      fixture.componentInstance.ngOnChanges(changes);
-    }
-
-    fixture.componentRef.injector.get(ChangeDetectorRef).detectChanges();
+    fixture.detectChanges();
   };
 
   const change = (changedProperties: Partial<SutType>) => {
@@ -229,6 +223,7 @@ export async function render<SutType, WrapperType = SutType>(
         fixture.nativeElement.removeAttribute('id');
       }
     }
+
     mountedFixtures.add(fixture);
 
     let isAlive = true;
@@ -338,7 +333,10 @@ function overrideChildComponentProviders(componentOverrides: ComponentOverride<a
 
 function hasOnChangesHook<SutType>(componentInstance: SutType): componentInstance is SutType & OnChanges {
   return (
-    'ngOnChanges' in componentInstance && typeof (componentInstance as SutType & OnChanges).ngOnChanges === 'function'
+    componentInstance !== null &&
+    typeof componentInstance === 'object' &&
+    'ngOnChanges' in componentInstance &&
+    typeof (componentInstance as SutType & OnChanges).ngOnChanges === 'function'
   );
 }
 

projects/testing-library/tests/changeInputs.spec.ts

@@ -1,4 +1,4 @@
-import { ChangeDetectionStrategy, Component, Input, OnChanges, SimpleChanges } from '@angular/core';
+import { ChangeDetectionStrategy, Component, Input, OnChanges } from '@angular/core';
 import { render, screen } from '../src/public_api';
 
 @Component({
@@ -40,30 +40,29 @@ test('changes the component with updated props while keeping other props untouch
 
 @Component({
   selector: 'atl-fixture',
-  template: ` {{ name }} `,
+  template: ` {{ propOne }} {{ propTwo }}`,
 })
 class FixtureWithNgOnChangesComponent implements OnChanges {
-  @Input() name = 'Sarah';
-  @Input() nameChanged?: (name: string, isFirstChange: boolean) => void;
-
-  ngOnChanges(changes: SimpleChanges) {
-    if (changes.name && this.nameChanged) {
-      this.nameChanged(changes.name.currentValue, changes.name.isFirstChange());
-    }
-  }
+  @Input() propOne = 'Init';
+  @Input() propTwo = '';
+
+  // eslint-disable-next-line @angular-eslint/no-empty-lifecycle-method, @typescript-eslint/no-empty-function
+  ngOnChanges() {}
 }
 
-test('will call ngOnChanges on change', async () => {
-  const nameChanged = jest.fn();
-  const componentInputs = { nameChanged };
-  const { changeInput } = await render(FixtureWithNgOnChangesComponent, { componentInputs });
-  expect(screen.getByText('Sarah')).toBeInTheDocument();
+test('calls ngOnChanges on change', async () => {
+  const componentInputs = { propOne: 'One', propTwo: 'Two' };
+  const { changeInput, fixture } = await render(FixtureWithNgOnChangesComponent, { componentInputs });
+  const spy = jest.spyOn(fixture.componentInstance, 'ngOnChanges');
+
+  expect(screen.getByText(`${componentInputs.propOne} ${componentInputs.propTwo}`)).toBeInTheDocument();
 
-  const name = 'Mark';
-  changeInput({ name });
+  const propOne = 'UpdatedOne';
+  const propTwo = 'UpdatedTwo';
+  changeInput({ propOne, propTwo });
 
-  expect(screen.getByText(name)).toBeInTheDocument();
-  expect(nameChanged).toHaveBeenCalledWith(name, false);
+  expect(spy).toHaveBeenCalledTimes(1);
+  expect(screen.getByText(`${propOne} ${propTwo}`)).toBeInTheDocument();
 });
 
 @Component({

projects/testing-library/tests/render.spec.ts

@@ -220,7 +220,7 @@ describe('Angular component life-cycle hooks', () => {
     expect(nameInitialized).toHaveBeenCalledWith('Initial');
   });
 
-  it('invokes ngOnChanges on initial render before ngOnInit', async () => {
+  it('invokes ngOnChanges with componentProperties on initial render before ngOnInit', async () => {
     const nameInitialized = jest.fn();
     const nameChanged = jest.fn();
     const componentProperties = { nameInitialized, nameChanged, name: 'Sarah' };
@@ -233,6 +233,23 @@ describe('Angular component life-cycle hooks', () => {
     expect(nameChanged).toHaveBeenCalledWith('Sarah', true);
     /// expect `nameChanged` to be called before `nameInitialized`
     expect(nameChanged.mock.invocationCallOrder[0]).toBeLessThan(nameInitialized.mock.invocationCallOrder[0]);
+    expect(nameChanged).toHaveBeenCalledTimes(1);
+  });
+
+  it('invokes ngOnChanges with componentInputs on initial render before ngOnInit', async () => {
+    const nameInitialized = jest.fn();
+    const nameChanged = jest.fn();
+    const componentInput = { nameInitialized, nameChanged, name: 'Sarah' };
+
+    const view = await render(FixtureWithNgOnChangesComponent, { componentInputs: componentInput });
+
+    /// We wish to test the utility function from `render` here.
+    // eslint-disable-next-line testing-library/prefer-screen-queries
+    expect(view.getByText('Sarah')).toBeInTheDocument();
+    expect(nameChanged).toHaveBeenCalledWith('Sarah', true);
+    /// expect `nameChanged` to be called before `nameInitialized`
+    expect(nameChanged.mock.invocationCallOrder[0]).toBeLessThan(nameInitialized.mock.invocationCallOrder[0]);
+    expect(nameChanged).toHaveBeenCalledTimes(1);
   });
 
   it('does not invoke ngOnChanges when no properties are provided', async () => {
@@ -243,30 +260,39 @@ describe('Angular component life-cycle hooks', () => {
       }
     }
 
-    await render(TestFixtureComponent);
+    const { fixture, detectChanges } = await render(TestFixtureComponent);
+    const spy = jest.spyOn(fixture.componentInstance, 'ngOnChanges');
+
+    detectChanges();
+
+    expect(spy).not.toHaveBeenCalled();
   });
 });
 
-test('waits for angular app initialization before rendering components', async () => {
-  const mock = jest.fn();
-
-  await render(FixtureComponent, {
-    providers: [
-      {
-        provide: APP_INITIALIZER,
-        useFactory: () => mock,
-        multi: true,
-      },
-    ],
-  });
+describe('initializer', () => {
+  it('waits for angular app initialization before rendering components', async () => {
+    const mock = jest.fn();
+
+    await render(FixtureComponent, {
+      providers: [
+        {
+          provide: APP_INITIALIZER,
+          useFactory: () => mock,
+          multi: true,
+        },
+      ],
+    });
 
-  expect(TestBed.inject(ApplicationInitStatus).done).toBe(true);
-  expect(mock).toHaveBeenCalled();
+    expect(TestBed.inject(ApplicationInitStatus).done).toBe(true);
+    expect(mock).toHaveBeenCalled();
+  });
 });
 
-test('gets the DebugElement', async () => {
-  const view = await render(FixtureComponent);
+describe('DebugElement', () => {
+  it('gets the DebugElement', async () => {
+    const view = await render(FixtureComponent);
 
-  expect(view.debugElement).not.toBeNull();
-  expect(view.debugElement.componentInstance).toBeInstanceOf(FixtureComponent);
+    expect(view.debugElement).not.toBeNull();
+    expect(view.debugElement.componentInstance).toBeInstanceOf(FixtureComponent);
+  });
 });