Skip to content

feat(testing-library): add an overload for template rendering #206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 22, 2021
Merged

feat(testing-library): add an overload for template rendering #206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions apps/example-app/src/app/examples/08-directive.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import { render, screen, fireEvent } from '@testing-library/angular';
import { SpoilerDirective } from './08-directive';

test('it is possible to test directives', async () => {
await render(SpoilerDirective, {
template: '<div appSpoiler data-testid="dir"></div>',
await render('<div appSpoiler data-testid="dir"></div>', {
declarations: [SpoilerDirective],
});

const directive = screen.getByTestId('dir');
Expand All @@ -25,8 +25,8 @@ test('it is possible to test directives with props', async () => {
const hidden = 'SPOILER ALERT';
const visible = 'There is nothing to see here ...';

await render(SpoilerDirective, {
template: '<div appSpoiler [hidden]="hidden" [visible]="visible"></div>',
await render('<div appSpoiler [hidden]="hidden" [visible]="visible"></div>', {
declarations: [SpoilerDirective],
componentProperties: {
hidden,
visible,
Expand All @@ -49,8 +49,8 @@ test('it is possible to test directives with props in template', async () => {
const hidden = 'SPOILER ALERT';
const visible = 'There is nothing to see here ...';

await render(SpoilerDirective, {
template: `<div appSpoiler hidden="${hidden}" visible="${visible}"></div>`,
await render(`<div appSpoiler hidden="${hidden}" visible="${visible}"></div>`, {
declarations: [SpoilerDirective],
});

expect(screen.queryByText(visible)).not.toBeInTheDocument();
Expand Down
26 changes: 26 additions & 0 deletions projects/testing-library/src/lib/models.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,9 @@ export interface RenderComponentOptions<ComponentType, Q extends Queries = typeo
removeAngularAttributes?: boolean;
}

/**
* @deprecated Use `render(template, { declarations: [SomeDirective] })` instead.
*/
// eslint-disable-next-line @typescript-eslint/ban-types
export interface RenderDirectiveOptions<WrapperType, Properties extends object = {}, Q extends Queries = typeof queries>
extends RenderComponentOptions<Properties, Q> {
Expand All @@ -262,6 +265,8 @@ export interface RenderDirectiveOptions<WrapperType, Properties extends object =
* const component = await render(SpoilerDirective, {
* template: `<div spoiler message='SPOILER'></div>`
* })
*
* @deprecated Use `render(template, { declarations: [SomeDirective] })` instead.
*/
template: string;
/**
Expand All @@ -282,6 +287,27 @@ export interface RenderDirectiveOptions<WrapperType, Properties extends object =
componentProperties?: Partial<WrapperType & Properties>;
}

// eslint-disable-next-line @typescript-eslint/ban-types
export interface RenderTemplateOptions<WrapperType, Properties extends object = {}, Q extends Queries = typeof queries>
extends RenderComponentOptions<Properties, Q> {
/**
* @description
* An Angular component to wrap the component in.
* The template will be overridden with the `template` option.
*
* @default
* `WrapperComponent`, an empty component that strips the `ng-version` attribute
*
* @example
* const component = await render(SpoilerDirective, {
* template: `<div spoiler message='SPOILER'></div>`
* wrapper: CustomWrapperComponent
* })
*/
wrapper?: Type<WrapperType>;
componentProperties?: Partial<WrapperType & Properties>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an opinion about the naming of componentProperties ?
Should it be better to name this wrapperComponentProperties or hostComponentProperties?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like wrapperComponentProperties which has consistency with wrapper.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I think it is hard to change the name now. Because RenderComponentOptions and RenderDirectiveOptions still has componentProperties, making overload with co-existing these are complex.

Copy link
Contributor Author

@lacolaco lacolaco Apr 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking of two options.

Option 1. Just properties or props in all type of render options

Vue or other families are adopting that. And it is not relevant directly, Storybook for Angular has also a similar API. This option brings breaking change for the entire package but the result is simple.

https://storybook.js.org/docs/angular/writing-stories/introduction

export default {
  title: 'Components/Button',
  component: Button,
} as Meta;

export const Primary = () => ({
  props: {
    label: 'Button',
    background: '#ff0',
  },
});

Option 2. Extending wrapper for passing wrapper component properties. Implementation for this is more complex than Option 1.

{
  wrapper: {
    component: WrapperComponent,
    properties: { ... }
  }
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like option 1, and you're (again) totally right.
Let's leave it with componentProperties, and we might rename that in a next breaking version.
I won't do it know, just because this is a minor difference.

}

export interface Config extends Pick<RenderComponentOptions<any>, 'excludeComponentDeclaration'> {
/**
* DOM Testing Library config
Expand Down
41 changes: 32 additions & 9 deletions projects/testing-library/src/lib/testing-library.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
waitForOptions as dtlWaitForOptions,
configure as dtlConfigure,
} from '@testing-library/dom';
import { RenderComponentOptions, RenderDirectiveOptions, RenderResult } from './models';
import { RenderComponentOptions, RenderDirectiveOptions, RenderTemplateOptions, RenderResult } from './models';
import { getConfig } from './config';

const mountedFixtures = new Set<ComponentFixture<any>>();
Expand All @@ -32,14 +32,24 @@ export async function render<ComponentType>(
component: Type<ComponentType>,
renderOptions?: RenderComponentOptions<ComponentType>,
): Promise<RenderResult<ComponentType, ComponentType>>;
/**
* @deprecated Use `render(template, { declarations: [DirectiveType] })` instead.
*/
export async function render<DirectiveType, WrapperType = WrapperComponent>(
component: Type<DirectiveType>,
renderOptions?: RenderDirectiveOptions<WrapperType>,
): Promise<RenderResult<WrapperType>>;
export async function render<WrapperType = WrapperComponent>(
template: string,
renderOptions?: RenderTemplateOptions<WrapperType>,
): Promise<RenderResult<WrapperType>>;

export async function render<SutType, WrapperType = SutType>(
sut: Type<SutType>,
renderOptions: RenderComponentOptions<SutType> | RenderDirectiveOptions<WrapperType> = {},
sut: Type<SutType> | string,
renderOptions:
| RenderComponentOptions<SutType>
| RenderDirectiveOptions<WrapperType>
| RenderTemplateOptions<WrapperType> = {},
): Promise<RenderResult<SutType>> {
const { dom: domConfig, ...globalConfig } = getConfig();
const {
Expand Down Expand Up @@ -69,7 +79,12 @@ export async function render<SutType, WrapperType = SutType>(
});

TestBed.configureTestingModule({
declarations: addAutoDeclarations(sut, { declarations, excludeComponentDeclaration, template, wrapper }),
declarations: addAutoDeclarations(sut, {
declarations,
excludeComponentDeclaration,
template,
wrapper,
}),
imports: addAutoImports({
imports: imports.concat(defaultImports),
routes,
Expand Down Expand Up @@ -176,7 +191,7 @@ export async function render<SutType, WrapperType = SutType>(
detectChanges,
navigate,
rerender,
debugElement: fixture.debugElement.query(By.directive(sut)),
debugElement: typeof sut === 'string' ? fixture.debugElement : fixture.debugElement.query(By.directive(sut)),
Copy link
Contributor Author

@lacolaco lacolaco Apr 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an only difference between render(directive, { template }) and render(template). Is it acceptable?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this fine.

container: fixture.nativeElement,
debug: (element = fixture.nativeElement, maxLength, options) =>
Array.isArray(element)
Expand All @@ -193,14 +208,18 @@ async function createComponent<SutType>(component: Type<SutType>): Promise<Compo
}

async function createComponentFixture<SutType>(
component: Type<SutType>,
sut: Type<SutType> | string,
{ template, wrapper }: Pick<RenderDirectiveOptions<any>, 'template' | 'wrapper'>,
): Promise<ComponentFixture<SutType>> {
if (typeof sut === 'string') {
TestBed.overrideTemplate(wrapper, sut);
return createComponent(wrapper);
}
if (template) {
TestBed.overrideTemplate(wrapper, template);
return createComponent(wrapper);
}
return createComponent(component);
return createComponent(sut);
}

function setComponentProperties<SutType>(
Expand Down Expand Up @@ -248,17 +267,21 @@ function getChangesObj<SutType>(oldProps: Partial<SutType> | null, newProps: Par
}

function addAutoDeclarations<SutType>(
component: Type<SutType>,
sut: Type<SutType> | string,
{
declarations,
excludeComponentDeclaration,
template,
wrapper,
}: Pick<RenderDirectiveOptions<any>, 'declarations' | 'excludeComponentDeclaration' | 'template' | 'wrapper'>,
) {
if (typeof sut === 'string') {
return [...declarations, wrapper];
}

const wrappers = () => (template ? [wrapper] : []);

const components = () => (excludeComponentDeclaration ? [] : [component]);
const components = () => (excludeComponentDeclaration ? [] : [sut]);

return [...declarations, ...wrappers(), ...components()];
}
Expand Down
60 changes: 45 additions & 15 deletions ...s/testing-library/tests/directive.spec.ts → ...ing-library/tests/render-template.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
/* eslint-disable testing-library/no-container */
/* eslint-disable testing-library/render-result-naming-convention */
import { Directive, HostListener, ElementRef, Input, Output, EventEmitter, Component } from '@angular/core';

import { render, fireEvent } from '../src/public_api';

@Directive({
// eslint-disable-next-line @angular-eslint/directive-selector
selector: '[onOff]',
})
export class OnOffDirective {
Expand All @@ -21,6 +24,7 @@ export class OnOffDirective {
}

@Directive({
// eslint-disable-next-line @angular-eslint/directive-selector
selector: '[update]',
})
export class UpdateInputDirective {
Expand All @@ -32,27 +36,53 @@ export class UpdateInputDirective {
constructor(private el: ElementRef) {}
}

@Component({
// eslint-disable-next-line @angular-eslint/component-selector
selector: 'greeting',
template: 'Hello {{ name }}!',
})
export class GreetingComponent {
@Input() name = 'World';
}

test('the directive renders', async () => {
const component = await render(OnOffDirective, {
template: '<div onOff></div>',
const component = await render('<div onOff></div>', {
declarations: [OnOffDirective],
});

expect(component.container.querySelector('[onoff]')).toBeInTheDocument();
});

test('uses the default props', async () => {
test('the component renders', async () => {
const component = await render('<greeting name="Angular"></greeting>', {
declarations: [GreetingComponent],
});

expect(component.container.querySelector('greeting')).toBeInTheDocument();
expect(component.getByText('Hello Angular!'));
});

test('the directive renders (compatibility with the deprecated signature)', async () => {
const component = await render(OnOffDirective, {
template: '<div onOff></div>',
});

expect(component.container.querySelector('[onoff]')).toBeInTheDocument();
});

test.only('uses the default props', async () => {
const component = await render('<div onOff></div>', {
declarations: [OnOffDirective],
});

fireEvent.click(component.getByText('init'));
fireEvent.click(component.getByText('on'));
fireEvent.click(component.getByText('off'));
});

test('overrides input properties', async () => {
const component = await render(OnOffDirective, {
template: '<div onOff on="hello"></div>',
const component = await render('<div onOff on="hello"></div>', {
declarations: [OnOffDirective],
});

fireEvent.click(component.getByText('init'));
Expand All @@ -62,8 +92,8 @@ test('overrides input properties', async () => {

test('overrides input properties via a wrapper', async () => {
// `bar` will be set as a property on the wrapper component, the property will be used to pass to the directive
const component = await render(OnOffDirective, {
template: '<div onOff [on]="bar"></div>',
const component = await render('<div onOff [on]="bar"></div>', {
declarations: [OnOffDirective],
componentProperties: {
bar: 'hello',
},
Expand All @@ -77,8 +107,8 @@ test('overrides input properties via a wrapper', async () => {
test('overrides output properties', async () => {
const clicked = jest.fn();

const component = await render(OnOffDirective, {
template: '<div onOff (clicked)="clicked($event)"></div>',
const component = await render('<div onOff (clicked)="clicked($event)"></div>', {
declarations: [OnOffDirective],
componentProperties: {
clicked,
},
Expand All @@ -93,8 +123,8 @@ test('overrides output properties', async () => {

describe('removeAngularAttributes', () => {
test('should remove angular attributes', async () => {
await render(OnOffDirective, {
template: '<div onOff (clicked)="clicked($event)"></div>',
await render('<div onOff (clicked)="clicked($event)"></div>', {
declarations: [OnOffDirective],
removeAngularAttributes: true,
});

Expand All @@ -103,8 +133,8 @@ describe('removeAngularAttributes', () => {
});

test('is disabled by default', async () => {
await render(OnOffDirective, {
template: '<div onOff (clicked)="clicked($event)"></div>',
await render('<div onOff (clicked)="clicked($event)"></div>', {
declarations: [OnOffDirective],
});

expect(document.querySelector('[ng-version]')).not.toBeNull();
Expand All @@ -113,8 +143,8 @@ describe('removeAngularAttributes', () => {
});

test('updates properties and invokes change detection', async () => {
const component = await render(UpdateInputDirective, {
template: '<div [update]="value" ></div>',
const component = await render('<div [update]="value" ></div>', {
declarations: [UpdateInputDirective],
componentProperties: {
value: 'value1',
},
Expand Down