From 9666dcefcbe72e0bcabd5bce549680abc1faa22b Mon Sep 17 00:00:00 2001 From: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com> Date: Mon, 27 Mar 2023 19:49:11 +0200 Subject: [PATCH 1/2] feat: remove change and changeInput in favor of rerender BREAKING CHANGE: Use erender instead of change. Use erender instead of changechangeInput For more info see https://github.com/testing-library/angular-testing-library/issues/365 --- .../src/app/issues/issue-222.spec.ts | 13 --- .../examples/16-input-getter-setter.spec.ts | 14 --- projects/testing-library/src/lib/models.ts | 16 --- .../src/lib/testing-library.ts | 28 ------ projects/testing-library/tests/change.spec.ts | 96 ------------------ .../tests/changeInputs.spec.ts | 97 ------------------- .../testing-library/tests/rerender.spec.ts | 36 +++++++ 7 files changed, 36 insertions(+), 264 deletions(-) delete mode 100644 projects/testing-library/tests/change.spec.ts delete mode 100644 projects/testing-library/tests/changeInputs.spec.ts diff --git a/apps/example-app-karma/src/app/issues/issue-222.spec.ts b/apps/example-app-karma/src/app/issues/issue-222.spec.ts index 17e9a02..5da35d4 100644 --- a/apps/example-app-karma/src/app/issues/issue-222.spec.ts +++ b/apps/example-app-karma/src/app/issues/issue-222.spec.ts @@ -13,16 +13,3 @@ it('https://github.com/testing-library/angular-testing-library/issues/222 with r expect(screen.getByText('Hello Mark')).toBeTruthy(); }); - -it('https://github.com/testing-library/angular-testing-library/issues/222 with change', async () => { - const { change } = await render(`
Hello {{ name}}
`, { - componentProperties: { - name: 'Sarah', - }, - }); - - expect(screen.getByText('Hello Sarah')).toBeTruthy(); - await change({ name: 'Mark' }); - - expect(screen.getByText('Hello Mark')).toBeTruthy(); -}); diff --git a/apps/example-app/src/app/examples/16-input-getter-setter.spec.ts b/apps/example-app/src/app/examples/16-input-getter-setter.spec.ts index 53dee01..4382d85 100644 --- a/apps/example-app/src/app/examples/16-input-getter-setter.spec.ts +++ b/apps/example-app/src/app/examples/16-input-getter-setter.spec.ts @@ -10,20 +10,6 @@ test('should run logic in the input setter and getter', async () => { expect(getterValueControl).toHaveTextContent('I am value from getter Angular'); }); -test('should run logic in the input setter and getter while changing', async () => { - const { change } = await render(InputGetterSetter, { componentProperties: { value: 'Angular' } }); - const valueControl = screen.getByTestId('value'); - const getterValueControl = screen.getByTestId('value-getter'); - - expect(valueControl).toHaveTextContent('I am value from setter Angular'); - expect(getterValueControl).toHaveTextContent('I am value from getter Angular'); - - await change({ value: 'React' }); - - expect(valueControl).toHaveTextContent('I am value from setter React'); - expect(getterValueControl).toHaveTextContent('I am value from getter React'); -}); - test('should run logic in the input setter and getter while re-rendering', async () => { const { rerender } = await render(InputGetterSetter, { componentProperties: { value: 'Angular' } }); diff --git a/projects/testing-library/src/lib/models.ts b/projects/testing-library/src/lib/models.ts index b4babe7..0a9b78a 100644 --- a/projects/testing-library/src/lib/models.ts +++ b/projects/testing-library/src/lib/models.ts @@ -63,22 +63,6 @@ export interface RenderResult extend 'componentProperties' | 'componentInputs' | 'componentOutputs' | 'detectChangesOnRender' >, ) => Promise; - /** - * @deprecated - * Use rerender instead. For more info see {@link https://github.com/testing-library/angular-testing-library/issues/365 GitHub Issue} - * - * @description - * Keeps the current fixture intact and invokes ngOnChanges with the updated properties. - */ - change: (changedProperties: Partial) => void; - /** - * @deprecated - * Use rerender instead. For more info see {@link https://github.com/testing-library/angular-testing-library/issues/365 GitHub Issue} - * - * @description - * Keeps the current fixture intact, update the @Input properties and invoke ngOnChanges with the updated properties. - */ - changeInput: (changedInputProperties: Partial) => void; } export interface RenderComponentOptions { diff --git a/projects/testing-library/src/lib/testing-library.ts b/projects/testing-library/src/lib/testing-library.ts index 2d492d6..5c6b0f2 100644 --- a/projects/testing-library/src/lib/testing-library.ts +++ b/projects/testing-library/src/lib/testing-library.ts @@ -162,32 +162,6 @@ export async function render( } }; - const changeInput = (changedInputProperties: Partial) => { - if (Object.keys(changedInputProperties).length === 0) { - return; - } - - setComponentInputs(fixture, changedInputProperties); - - fixture.detectChanges(); - }; - - const change = (changedProperties: Partial) => { - if (Object.keys(changedProperties).length === 0) { - return; - } - - const changes = getChangesObj(fixture.componentInstance as Record, changedProperties); - - setComponentProperties(fixture, changedProperties); - - if (hasOnChangesHook(fixture.componentInstance)) { - fixture.componentInstance.ngOnChanges(changes); - } - - fixture.componentRef.injector.get(ChangeDetectorRef).detectChanges(); - }; - const navigate = async (elementOrPath: Element | string, basePath = ''): Promise => { const href = typeof elementOrPath === 'string' ? elementOrPath : elementOrPath.getAttribute('href'); const [path, params] = (basePath + href).split('?'); @@ -234,8 +208,6 @@ export async function render( detectChanges: () => detectChanges(), navigate, rerender, - change, - changeInput, // @ts-ignore: fixture assigned debugElement: fixture.debugElement, // @ts-ignore: fixture assigned diff --git a/projects/testing-library/tests/change.spec.ts b/projects/testing-library/tests/change.spec.ts deleted file mode 100644 index d6d30f4..0000000 --- a/projects/testing-library/tests/change.spec.ts +++ /dev/null @@ -1,96 +0,0 @@ -import { ChangeDetectionStrategy, Component, Input, OnChanges } from '@angular/core'; -import { render, screen } from '../src/public_api'; - -@Component({ - selector: 'atl-fixture', - template: ` {{ firstName }} {{ lastName }} `, -}) -class FixtureComponent { - @Input() firstName = 'Sarah'; - @Input() lastName?: string; -} - -test('changes the component with updated props', async () => { - const { change } = await render(FixtureComponent); - expect(screen.getByText('Sarah')).toBeInTheDocument(); - - const firstName = 'Mark'; - change({ firstName }); - - expect(screen.getByText(firstName)).toBeInTheDocument(); -}); - -test('changes the component with updated props while keeping other props untouched', async () => { - const firstName = 'Mark'; - const lastName = 'Peeters'; - const { change } = await render(FixtureComponent, { - componentProperties: { - firstName, - lastName, - }, - }); - - expect(screen.getByText(`${firstName} ${lastName}`)).toBeInTheDocument(); - - const firstName2 = 'Chris'; - change({ firstName: firstName2 }); - - expect(screen.getByText(`${firstName2} ${lastName}`)).toBeInTheDocument(); -}); - -@Component({ - selector: 'atl-fixture', - template: ` {{ propOne }} {{ propTwo }}`, -}) -class FixtureWithNgOnChangesComponent implements OnChanges { - @Input() propOne = 'Init'; - @Input() propTwo = ''; - - // eslint-disable-next-line @angular-eslint/no-empty-lifecycle-method, @typescript-eslint/no-empty-function - ngOnChanges() {} -} - -test('calls ngOnChanges on change', async () => { - const componentInputs = { propOne: 'One', propTwo: 'Two' }; - const { change, fixture } = await render(FixtureWithNgOnChangesComponent, { componentInputs }); - const spy = jest.spyOn(fixture.componentInstance, 'ngOnChanges'); - - expect(screen.getByText(`${componentInputs.propOne} ${componentInputs.propTwo}`)).toBeInTheDocument(); - - const propOne = 'UpdatedOne'; - const propTwo = 'UpdatedTwo'; - change({ propOne, propTwo }); - - expect(spy).toHaveBeenCalledTimes(1); - expect(screen.getByText(`${propOne} ${propTwo}`)).toBeInTheDocument(); -}); - -test('does not invoke ngOnChanges on change without props', async () => { - const componentInputs = { propOne: 'One', propTwo: 'Two' }; - const { change, fixture } = await render(FixtureWithNgOnChangesComponent, { componentInputs }); - const spy = jest.spyOn(fixture.componentInstance, 'ngOnChanges'); - - expect(screen.getByText(`${componentInputs.propOne} ${componentInputs.propTwo}`)).toBeInTheDocument(); - - change({}); - expect(spy).not.toHaveBeenCalled(); - - expect(screen.getByText(`${componentInputs.propOne} ${componentInputs.propTwo}`)).toBeInTheDocument(); -}); -@Component({ - changeDetection: ChangeDetectionStrategy.OnPush, - selector: 'atl-fixture', - template: `
Number
`, -}) -class FixtureWithOnPushComponent { - @Input() activeField = ''; -} - -test('update properties on change', async () => { - const { change } = await render(FixtureWithOnPushComponent); - const numberHtmlElementRef = screen.queryByTestId('number'); - - expect(numberHtmlElementRef).not.toHaveClass('active'); - change({ activeField: 'number' }); - expect(numberHtmlElementRef).toHaveClass('active'); -}); diff --git a/projects/testing-library/tests/changeInputs.spec.ts b/projects/testing-library/tests/changeInputs.spec.ts deleted file mode 100644 index 8a97082..0000000 --- a/projects/testing-library/tests/changeInputs.spec.ts +++ /dev/null @@ -1,97 +0,0 @@ -import { ChangeDetectionStrategy, Component, Input, OnChanges } from '@angular/core'; -import { render, screen } from '../src/public_api'; - -@Component({ - selector: 'atl-fixture', - template: ` {{ firstName }} {{ lastName }} `, -}) -class FixtureComponent { - @Input() firstName = 'Sarah'; - @Input() lastName?: string; -} - -test('changes the component with updated props', async () => { - const { changeInput } = await render(FixtureComponent); - expect(screen.getByText('Sarah')).toBeInTheDocument(); - - const firstName = 'Mark'; - changeInput({ firstName }); - - expect(screen.getByText(firstName)).toBeInTheDocument(); -}); - -test('changes the component with updated props while keeping other props untouched', async () => { - const firstName = 'Mark'; - const lastName = 'Peeters'; - const { changeInput } = await render(FixtureComponent, { - componentInputs: { - firstName, - lastName, - }, - }); - - expect(screen.getByText(`${firstName} ${lastName}`)).toBeInTheDocument(); - - const firstName2 = 'Chris'; - changeInput({ firstName: firstName2 }); - - expect(screen.getByText(`${firstName2} ${lastName}`)).toBeInTheDocument(); -}); - -@Component({ - selector: 'atl-fixture', - template: ` {{ propOne }} {{ propTwo }}`, -}) -class FixtureWithNgOnChangesComponent implements OnChanges { - @Input() propOne = 'Init'; - @Input() propTwo = ''; - - // eslint-disable-next-line @angular-eslint/no-empty-lifecycle-method, @typescript-eslint/no-empty-function - ngOnChanges() {} -} - -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 propOne = 'UpdatedOne'; - const propTwo = 'UpdatedTwo'; - changeInput({ propOne, propTwo }); - - expect(spy).toHaveBeenCalledTimes(1); - expect(screen.getByText(`${propOne} ${propTwo}`)).toBeInTheDocument(); -}); - -test('does not invoke ngOnChanges on change without props', 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(); - - changeInput({}); - expect(spy).not.toHaveBeenCalled(); - - expect(screen.getByText(`${componentInputs.propOne} ${componentInputs.propTwo}`)).toBeInTheDocument(); -}); - -@Component({ - changeDetection: ChangeDetectionStrategy.OnPush, - selector: 'atl-fixture', - template: `
Number
`, -}) -class FixtureWithOnPushComponent { - @Input() activeField = ''; -} - -test('update properties on change', async () => { - const { changeInput } = await render(FixtureWithOnPushComponent); - const numberHtmlElementRef = screen.queryByTestId('number'); - - expect(numberHtmlElementRef).not.toHaveClass('active'); - changeInput({ activeField: 'number' }); - expect(numberHtmlElementRef).toHaveClass('active'); -}); diff --git a/projects/testing-library/tests/rerender.spec.ts b/projects/testing-library/tests/rerender.spec.ts index a06beaf..2e5ee4c 100644 --- a/projects/testing-library/tests/rerender.spec.ts +++ b/projects/testing-library/tests/rerender.spec.ts @@ -35,6 +35,7 @@ test('rerenders without props', async () => { await rerender(); expect(screen.getByText('Sarah')).toBeInTheDocument(); + expect(ngOnChangesSpy).toHaveBeenCalledTimes(1); // one time initially and one time for rerender }); test('rerenders the component with updated inputs', async () => { @@ -48,6 +49,41 @@ test('rerenders the component with updated inputs', async () => { }); test('rerenders the component with updated props and resets other props', async () => { + const firstName = 'Mark'; + const lastName = 'Peeters'; + const { rerender } = await render(FixtureComponent, { + componentInputs: { + firstName, + lastName, + }, + }); + + expect(screen.getByText(`${firstName} ${lastName}`)).toBeInTheDocument(); + + const firstName2 = 'Chris'; + await rerender({ componentInputs: { firstName: firstName2 } }); + + 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; + expect(rerenderedChanges).toEqual({ + lastName: { + previousValue: 'Peeters', + currentValue: undefined, + firstChange: false, + }, + firstName: { + previousValue: 'Mark', + currentValue: 'Chris', + firstChange: false, + }, + }); +}); + +test('rerenders the component with updated props and resets other props with componentProperties', async () => { const firstName = 'Mark'; const lastName = 'Peeters'; const { rerender } = await render(FixtureComponent, { From 33d0069ed2829149402dbb1122224e3cdff18079 Mon Sep 17 00:00:00 2001 From: Torsten Knauf Date: Mon, 3 Apr 2023 19:28:57 +0200 Subject: [PATCH 2/2] fix: calculate changes for input properties like component properties (#380) --- .../src/lib/testing-library.ts | 36 ++++++++++--------- .../testing-library/tests/rerender.spec.ts | 2 +- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/projects/testing-library/src/lib/testing-library.ts b/projects/testing-library/src/lib/testing-library.ts index 5c6b0f2..66cb819 100644 --- a/projects/testing-library/src/lib/testing-library.ts +++ b/projects/testing-library/src/lib/testing-library.ts @@ -133,12 +133,7 @@ export async function render( >, ) => { 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); + const changesInComponentInput = update(fixture, renderedInputKeys, newComponentInputs, setComponentInputs); renderedInputKeys = Object.keys(newComponentInputs); const newComponentOutputs = properties?.componentOutputs ?? {}; @@ -151,11 +146,15 @@ export async function render( renderedOutputKeys = Object.keys(newComponentOutputs); const newComponentProps = properties?.componentProperties ?? {}; - const changes = updateProps(fixture, renderedPropKeys, newComponentProps); + const changesInComponentProps = update(fixture, renderedPropKeys, newComponentProps, setComponentProperties); + renderedPropKeys = Object.keys(newComponentProps); + if (hasOnChangesHook(fixture.componentInstance)) { - fixture.componentInstance.ngOnChanges(changes); + fixture.componentInstance.ngOnChanges({ + ...changesInComponentInput, + ...changesInComponentProps, + }); } - renderedPropKeys = Object.keys(newComponentProps); if (properties?.detectChangesOnRender !== false) { fixture.componentRef.injector.get(ChangeDetectorRef).detectChanges(); @@ -361,27 +360,32 @@ function getChangesObj(oldProps: Record | null, newProps: Record( +function update( fixture: ComponentFixture, - prevRenderedPropsKeys: string[], - newProps: Record, + prevRenderedKeys: string[], + newValues: Record, + updateFunction: ( + fixture: ComponentFixture, + values: RenderTemplateOptions['componentInputs' | 'componentProperties'], + ) => void, ) { const componentInstance = fixture.componentInstance as Record; const simpleChanges: SimpleChanges = {}; - for (const key of prevRenderedPropsKeys) { - if (!Object.prototype.hasOwnProperty.call(newProps, key)) { + for (const key of prevRenderedKeys) { + if (!Object.prototype.hasOwnProperty.call(newValues, key)) { simpleChanges[key] = new SimpleChange(componentInstance[key], undefined, false); delete componentInstance[key]; } } - for (const [key, value] of Object.entries(newProps)) { + for (const [key, value] of Object.entries(newValues)) { if (value !== componentInstance[key]) { simpleChanges[key] = new SimpleChange(componentInstance[key], value, false); } } - setComponentProperties(fixture, newProps); + + updateFunction(fixture, newValues); return simpleChanges; } diff --git a/projects/testing-library/tests/rerender.spec.ts b/projects/testing-library/tests/rerender.spec.ts index 2e5ee4c..9c25257 100644 --- a/projects/testing-library/tests/rerender.spec.ts +++ b/projects/testing-library/tests/rerender.spec.ts @@ -48,7 +48,7 @@ test('rerenders the component with updated inputs', async () => { expect(screen.getByText(firstName)).toBeInTheDocument(); }); -test('rerenders the component with updated props and resets other props', async () => { +test('rerenders the component with updated inputs and resets other props', async () => { const firstName = 'Mark'; const lastName = 'Peeters'; const { rerender } = await render(FixtureComponent, {