From e4acbbdc5dde190195379ff2e428608893dfdbdd Mon Sep 17 00:00:00 2001 From: shaman-apprentice Date: Sat, 11 Feb 2023 22:24:50 +0100 Subject: [PATCH 1/4] feat: support ngOnChanges with correct simple change object within rerender ref #365 --- projects/testing-library/src/lib/models.ts | 2 +- .../src/lib/testing-library.ts | 59 +++++++++++++++++-- .../testing-library/tests/rerender.spec.ts | 28 ++++++++- 3 files changed, 81 insertions(+), 8 deletions(-) diff --git a/projects/testing-library/src/lib/models.ts b/projects/testing-library/src/lib/models.ts index 17974c6..3580a0b 100644 --- a/projects/testing-library/src/lib/models.ts +++ b/projects/testing-library/src/lib/models.ts @@ -55,7 +55,7 @@ export interface RenderResult extend /** * @description * Re-render the same component with different properties. - * This creates a new instance of the component. + * Properties not passed in again are removed. */ rerender: ( properties?: Pick< diff --git a/projects/testing-library/src/lib/testing-library.ts b/projects/testing-library/src/lib/testing-library.ts index d2cf3ad..135d113 100644 --- a/projects/testing-library/src/lib/testing-library.ts +++ b/projects/testing-library/src/lib/testing-library.ts @@ -123,14 +123,38 @@ export async function render( await renderFixture(componentProperties, componentInputs, componentOutputs); + let renderedPropKeys = Object.keys(componentProperties); + let renderedInputKeys = Object.keys(componentInputs); + let renderedOutputKeys = Object.keys(componentOutputs); const rerender = async ( properties?: Pick, 'componentProperties' | 'componentInputs' | 'componentOutputs'>, ) => { - await renderFixture( - properties?.componentProperties ?? {}, - properties?.componentInputs ?? {}, - properties?.componentOutputs ?? {}, - ); + const newComponentInputs = properties?.componentInputs ?? {}; + for (const inputKey of renderedInputKeys) { + if (!Object.prototype.hasOwnProperty.call(newComponentInputs, inputKey)) { + delete (fixture.componentInstance as any)[inputKey]; + } + } + setComponentInputs(fixture, newComponentInputs); + renderedInputKeys = Object.keys(newComponentInputs); + + const newComponentOutputs = properties?.componentOutputs ?? {}; + for (const outputKey of renderedOutputKeys) { + if (!Object.prototype.hasOwnProperty.call(newComponentOutputs, outputKey)) { + delete (fixture.componentInstance as any)[outputKey]; + } + } + setComponentOutputs(fixture, newComponentOutputs); + renderedOutputKeys = Object.keys(newComponentOutputs); + + const newComponentProps = properties?.componentProperties ?? {}; + const changes = updateProps(fixture, renderedPropKeys, newComponentProps); + if (hasOnChangesHook(fixture.componentInstance)) { + fixture.componentInstance.ngOnChanges(changes); + } + renderedPropKeys = Object.keys(newComponentProps); + + fixture.componentRef.injector.get(ChangeDetectorRef).detectChanges(); }; const changeInput = (changedInputProperties: Partial) => { @@ -360,6 +384,31 @@ function getChangesObj(oldProps: Record | null, newProps: Record( + fixture: ComponentFixture, + prevRenderedPropsKeys: string[], + newProps: Record, +) { + const componentInstance = fixture.componentInstance as Record; + const simpleChanges: SimpleChanges = {}; + + for (const key of prevRenderedPropsKeys) { + if (!Object.prototype.hasOwnProperty.call(newProps, key)) { + simpleChanges[key] = new SimpleChange(componentInstance[key], undefined, false); + delete componentInstance[key]; + } + } + + for (const [key, value] of Object.entries(newProps)) { + if (value !== componentInstance[key]) { + simpleChanges[key] = new SimpleChange(componentInstance[key], value, false); + } + } + setComponentProperties(fixture, newProps); + + return simpleChanges; +} + function addAutoDeclarations( sut: Type | string, { diff --git a/projects/testing-library/tests/rerender.spec.ts b/projects/testing-library/tests/rerender.spec.ts index d0ee43b..ca95fa4 100644 --- a/projects/testing-library/tests/rerender.spec.ts +++ b/projects/testing-library/tests/rerender.spec.ts @@ -1,15 +1,23 @@ -import { Component, Input } from '@angular/core'; +import { Component, Input, OnChanges, SimpleChanges } from '@angular/core'; import { render, screen } from '../src/public_api'; +let ngOnChangesSpy: jest.Mock; @Component({ selector: 'atl-fixture', template: ` {{ firstName }} {{ lastName }} `, }) -class FixtureComponent { +class FixtureComponent implements OnChanges { @Input() firstName = 'Sarah'; @Input() lastName?: string; + ngOnChanges(changes: SimpleChanges): void { + ngOnChangesSpy(changes); + } } +beforeEach(() => { + ngOnChangesSpy = jest.fn(); +}); + test('rerenders the component with updated props', async () => { const { rerender } = await render(FixtureComponent); expect(screen.getByText('Sarah')).toBeInTheDocument(); @@ -54,6 +62,22 @@ test('rerenders the component with updated props and resets other props', async const firstName2 = 'Chris'; await rerender({ componentProperties: { firstName: firstName2 } }); + expect(screen.getByText(`${firstName2}`)).toBeInTheDocument(); expect(screen.queryByText(`${firstName2} ${lastName}`)).not.toBeInTheDocument(); expect(screen.queryByText(`${firstName} ${lastName}`)).not.toBeInTheDocument(); + + expect(ngOnChangesSpy).toHaveBeenCalledTimes(2); // one time initially and one time for rerender + const rerenderedChanges = ngOnChangesSpy.mock.calls[1][0] as SimpleChanges; + expect(rerenderedChanges).toEqual({ + lastName: { + previousValue: 'Peeters', + currentValue: undefined, + firstChange: false, + }, + firstName: { + previousValue: 'Mark', + currentValue: 'Chris', + firstChange: false, + }, + }); }); From 66ba0c79048c831d5bd3836dc3db335f3b8e19d8 Mon Sep 17 00:00:00 2001 From: Torsten Knauf Date: Tue, 21 Feb 2023 21:26:04 +0100 Subject: [PATCH 2/4] Update projects/testing-library/tests/rerender.spec.ts Co-authored-by: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com> --- projects/testing-library/tests/rerender.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/projects/testing-library/tests/rerender.spec.ts b/projects/testing-library/tests/rerender.spec.ts index ca95fa4..bd1a063 100644 --- a/projects/testing-library/tests/rerender.spec.ts +++ b/projects/testing-library/tests/rerender.spec.ts @@ -62,9 +62,9 @@ test('rerenders the component with updated props and resets other props', async const firstName2 = 'Chris'; await rerender({ componentProperties: { firstName: firstName2 } }); - expect(screen.getByText(`${firstName2}`)).toBeInTheDocument(); - expect(screen.queryByText(`${firstName2} ${lastName}`)).not.toBeInTheDocument(); - expect(screen.queryByText(`${firstName} ${lastName}`)).not.toBeInTheDocument(); + expect(screen.getByText(firstName2)).toBeInTheDocument(); + expect(screen.queryByText(firstName)).not.toBeInTheDocument(); + expect(screen.queryByText(lastName)).not.toBeInTheDocument(); expect(ngOnChangesSpy).toHaveBeenCalledTimes(2); // one time initially and one time for rerender const rerenderedChanges = ngOnChangesSpy.mock.calls[1][0] as SimpleChanges; From 4c5a2552e54d73a75d3217c89a35ae0a4bb16655 Mon Sep 17 00:00:00 2001 From: shaman-apprentice Date: Sun, 5 Mar 2023 18:59:25 +0100 Subject: [PATCH 3/4] feat: support detectChangesOnRender within rerender ref #365 --- projects/testing-library/src/lib/models.ts | 2 +- .../src/lib/testing-library.ts | 12 +++++- .../testing-library/tests/rerender.spec.ts | 37 +++++++++++++++++++ 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/projects/testing-library/src/lib/models.ts b/projects/testing-library/src/lib/models.ts index 3580a0b..761141b 100644 --- a/projects/testing-library/src/lib/models.ts +++ b/projects/testing-library/src/lib/models.ts @@ -60,7 +60,7 @@ export interface RenderResult extend rerender: ( properties?: Pick< RenderTemplateOptions, - 'componentProperties' | 'componentInputs' | 'componentOutputs' + 'componentProperties' | 'componentInputs' | 'componentOutputs' | 'detectChangesOnRender' >, ) => Promise; /** diff --git a/projects/testing-library/src/lib/testing-library.ts b/projects/testing-library/src/lib/testing-library.ts index 135d113..0e460b8 100644 --- a/projects/testing-library/src/lib/testing-library.ts +++ b/projects/testing-library/src/lib/testing-library.ts @@ -127,7 +127,10 @@ export async function render( let renderedInputKeys = Object.keys(componentInputs); let renderedOutputKeys = Object.keys(componentOutputs); const rerender = async ( - properties?: Pick, 'componentProperties' | 'componentInputs' | 'componentOutputs'>, + properties?: Pick< + RenderTemplateOptions, + 'componentProperties' | 'componentInputs' | 'componentOutputs' | 'detectChangesOnRender' + >, ) => { const newComponentInputs = properties?.componentInputs ?? {}; for (const inputKey of renderedInputKeys) { @@ -154,7 +157,12 @@ export async function render( } renderedPropKeys = Object.keys(newComponentProps); - fixture.componentRef.injector.get(ChangeDetectorRef).detectChanges(); + if ( + properties?.detectChangesOnRender === true || + (properties?.detectChangesOnRender === undefined && detectChangesOnRender === true) + ) { + fixture.componentRef.injector.get(ChangeDetectorRef).detectChanges(); + } }; const changeInput = (changedInputProperties: Partial) => { diff --git a/projects/testing-library/tests/rerender.spec.ts b/projects/testing-library/tests/rerender.spec.ts index bd1a063..cde6c82 100644 --- a/projects/testing-library/tests/rerender.spec.ts +++ b/projects/testing-library/tests/rerender.spec.ts @@ -81,3 +81,40 @@ test('rerenders the component with updated props and resets other props', async }, }); }); + +describe('detectChangesOnRender', () => { + test('change detection gets not called if disabled within render', async () => { + const { rerender, detectChanges } = await render(FixtureComponent, { detectChangesOnRender: false }); + detectChanges(); + expect(screen.getByText('Sarah')).toBeInTheDocument(); + + const firstName = 'Mark'; + await rerender({ componentInputs: { firstName } }); + + expect(screen.getByText('Sarah')).toBeInTheDocument(); + expect(screen.queryByText(firstName)).not.toBeInTheDocument(); + }); + + test('change detection gets called if disabled within render but enabled within rerender', async () => { + const { rerender, detectChanges } = await render(FixtureComponent, { detectChangesOnRender: false }); + detectChanges(); + expect(screen.getByText('Sarah')).toBeInTheDocument(); + + const firstName = 'Mark'; + await rerender({ componentInputs: { firstName }, detectChangesOnRender: true }); + + expect(screen.getByText(firstName)).toBeInTheDocument(); + expect(screen.queryByText('Sarah')).not.toBeInTheDocument(); + }); + + test('change detection gets not called if disabled within rerender', async () => { + const { rerender } = await render(FixtureComponent); + expect(screen.getByText('Sarah')).toBeInTheDocument(); + + const firstName = 'Mark'; + await rerender({ componentInputs: { firstName }, detectChangesOnRender: false }); + + expect(screen.getByText('Sarah')).toBeInTheDocument(); + expect(screen.queryByText(firstName)).not.toBeInTheDocument(); + }); +}); From d2189079fdb1e3a1d7903d1d7872722fa66c3985 Mon Sep 17 00:00:00 2001 From: shaman-apprentice Date: Fri, 10 Mar 2023 15:58:34 +0100 Subject: [PATCH 4/4] feat: only check for own detectChangesOnRender flag ref #365 --- .../src/lib/testing-library.ts | 5 +-- .../testing-library/tests/rerender.spec.ts | 40 ++++--------------- 2 files changed, 8 insertions(+), 37 deletions(-) diff --git a/projects/testing-library/src/lib/testing-library.ts b/projects/testing-library/src/lib/testing-library.ts index 0e460b8..2d492d6 100644 --- a/projects/testing-library/src/lib/testing-library.ts +++ b/projects/testing-library/src/lib/testing-library.ts @@ -157,10 +157,7 @@ export async function render( } renderedPropKeys = Object.keys(newComponentProps); - if ( - properties?.detectChangesOnRender === true || - (properties?.detectChangesOnRender === undefined && detectChangesOnRender === true) - ) { + if (properties?.detectChangesOnRender !== false) { fixture.componentRef.injector.get(ChangeDetectorRef).detectChanges(); } }; diff --git a/projects/testing-library/tests/rerender.spec.ts b/projects/testing-library/tests/rerender.spec.ts index cde6c82..a06beaf 100644 --- a/projects/testing-library/tests/rerender.spec.ts +++ b/projects/testing-library/tests/rerender.spec.ts @@ -82,39 +82,13 @@ test('rerenders the component with updated props and resets other props', async }); }); -describe('detectChangesOnRender', () => { - test('change detection gets not called if disabled within render', async () => { - const { rerender, detectChanges } = await render(FixtureComponent, { detectChangesOnRender: false }); - detectChanges(); - expect(screen.getByText('Sarah')).toBeInTheDocument(); - - const firstName = 'Mark'; - await rerender({ componentInputs: { firstName } }); - - expect(screen.getByText('Sarah')).toBeInTheDocument(); - expect(screen.queryByText(firstName)).not.toBeInTheDocument(); - }); - - test('change detection gets called if disabled within render but enabled within rerender', async () => { - const { rerender, detectChanges } = await render(FixtureComponent, { detectChangesOnRender: false }); - detectChanges(); - expect(screen.getByText('Sarah')).toBeInTheDocument(); - - const firstName = 'Mark'; - await rerender({ componentInputs: { firstName }, detectChangesOnRender: true }); - - expect(screen.getByText(firstName)).toBeInTheDocument(); - expect(screen.queryByText('Sarah')).not.toBeInTheDocument(); - }); - - test('change detection gets not called if disabled within rerender', async () => { - const { rerender } = await render(FixtureComponent); - expect(screen.getByText('Sarah')).toBeInTheDocument(); +test('change detection gets not called if `detectChangesOnRender` is set to false', async () => { + const { rerender } = await render(FixtureComponent); + expect(screen.getByText('Sarah')).toBeInTheDocument(); - const firstName = 'Mark'; - await rerender({ componentInputs: { firstName }, detectChangesOnRender: false }); + const firstName = 'Mark'; + await rerender({ componentInputs: { firstName }, detectChangesOnRender: false }); - expect(screen.getByText('Sarah')).toBeInTheDocument(); - expect(screen.queryByText(firstName)).not.toBeInTheDocument(); - }); + expect(screen.getByText('Sarah')).toBeInTheDocument(); + expect(screen.queryByText(firstName)).not.toBeInTheDocument(); });