Skip to content
This repository has been archived by the owner on Oct 1, 2024. It is now read-only.

Enable/fix test typing #2034

Merged
merged 14 commits into from
Sep 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion packages/react-async/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

<!-- ## Unreleased -->
## Unreleased

### Changed

- Enable type checking in tests and fix type errors. [[#2034](https://github.com/Shopify/quilt/pull/2034)]

## 4.1.5 - 2021-08-30

Expand Down
1 change: 1 addition & 0 deletions packages/react-async/src/context/tests/assets.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ describe('AsyncAssetManager', () => {
asyncAssets.markAsUsed(id);
asyncAssets.effect.betweenEachPass!({
finished: false,
cancelled: false,
index: 0,
renderDuration: 0,
resolveDuration: 0,
Expand Down
4 changes: 2 additions & 2 deletions packages/react-async/src/tests/hooks.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,6 @@ function createAsyncComponentType({
return AsyncComponent as any;
}

function noop() {
return undefined;
function noop(_?: {}) {
return () => {};
}
1 change: 0 additions & 1 deletion packages/react-async/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
"./src/**/*.ts",
"./src/**/*.tsx"
],
"exclude": ["**/test/**/*", "**/tests/**/*"],
"references": [
{"path": "../async"},
{"path": "../react-effect"},
Expand Down
6 changes: 5 additions & 1 deletion packages/react-bugsnag/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

<!-- ## Unreleased -->
## Unreleased

### Changed

- Enable type checking in tests and fix type errors. [[#2034](https://github.com/Shopify/quilt/pull/2034)]

## 2.2.3 - 2021-08-24

Expand Down
2 changes: 0 additions & 2 deletions packages/react-bugsnag/src/tests/client.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import ReactPlugin from '@bugsnag/plugin-react';

import {createBugsnagClient} from '../client';

jest.mock('@bugsnag/js', () => ({
Expand Down
3 changes: 1 addition & 2 deletions packages/react-bugsnag/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,5 @@
"../../config/typescript/*.d.ts",
"./src/**/*.ts",
"./src/**/*.tsx"
],
"exclude": ["**/test/**/*", "**/tests/**/*"]
]
}
6 changes: 5 additions & 1 deletion packages/react-form/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

<!-- ## Unreleased -->
## Unreleased

### Changed

- Enable type checking in tests and fix type errors. [[#2034](https://github.com/Shopify/quilt/pull/2034)]

## 1.1.3 - 2021-08-24

Expand Down
10 changes: 5 additions & 5 deletions packages/react-form/src/hooks/field/tests/field.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -276,12 +276,12 @@ describe('useField', () => {
wrapper.find('input')!.trigger('onChange', changeEvent(newValue));
wrapper.find('input')!.trigger('onBlur', blurEvent());

const allErrorsFirstValidation = wrapper.find('p').props.children;
const allErrorsFirstValidation = wrapper.find('p')!.props.children;

wrapper.find('input')!.trigger('onChange', changeEvent(newValue));
wrapper.find('input')!.trigger('onBlur', blurEvent());

const allErrorsSecondValidation = wrapper.find('p').props.children;
const allErrorsSecondValidation = wrapper.find('p')!.props.children;

expect(allErrorsFirstValidation).toStrictEqual(
allErrorsSecondValidation,
Expand Down Expand Up @@ -318,17 +318,17 @@ describe('useField', () => {
const wrapper = mount(<TestField config={fieldConfig} />);
const newValue = faker.commerce.product();

expect(wrapper.find('p').props.children).toStrictEqual([]);
expect(wrapper.find('p')!.props.children).toStrictEqual([]);

wrapper.find('input')!.trigger('onChange', changeEvent(newValue));
wrapper.find('input')!.trigger('onBlur', blurEvent());

const allErrorsFirstValidation = wrapper.find('p').props.children;
const allErrorsFirstValidation = wrapper.find('p')!.props.children;

wrapper.find('input')!.trigger('onChange', changeEvent(newValue));
wrapper.find('input')!.trigger('onBlur', blurEvent());

const allErrorsSecondValidation = wrapper.find('p').props.children;
const allErrorsSecondValidation = wrapper.find('p')!.props.children;

expect(allErrorsFirstValidation).toBe(allErrorsSecondValidation);
});
Expand Down
6 changes: 3 additions & 3 deletions packages/react-form/src/hooks/list/tests/baselist.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -797,7 +797,7 @@ describe('useBaseList', () => {
</>
))}

<button type="button" onClick={onNewDefault} />
<button type="button" onClick={onNewDefault as any} />
Copy link
Member

Choose a reason for hiding this comment

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

Having to cast this as any seems odd. How come it's needed? Is there a way we can change the signature of onNewDefault to make it compliant?

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 chose the brute force cast because the test doesn't deal with MouseEvents and I wanted to minimize changes to the original test. I followed the example of clickEvent() and changeEvent() used throughout react-forms tests. I agree it seems wonky, but I'm not sure how to avoid it without replacing Variant[] values with fake React.MouseEvent objects.

Copy link
Member

Choose a reason for hiding this comment

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

Uuugh yeah, it seems that this is doing weird things - it's triggering a button onClick with an argument that is nothing to do with a MouseEvent I'm kinda surprised that this works at all. This shouldn't be using a button - is should be some custom Event type that gets triggered and then we can control the type rather than using something weird and not fit for purpose .

I guess this is fine for now - but at some point we should work out how to rework this to use a custom event rather than repurposing the onClick event

</>
);
}
Expand Down Expand Up @@ -847,7 +847,7 @@ describe('useBaseList', () => {
optionName: newDefaultOption,
optionValue: newDefaultOptionValue,
},
]);
] as any);
Copy link
Member

Choose a reason for hiding this comment

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

Having to cast this as any seems odd. How come it's needed? Is there a way we can change the signature of newDefaultPrice/newDefaultOption/newDefaultOptionValue to make it compliant?


expect(wrapper).toContainReactText(`Default: ${newDefaultPrice}`);
expect(wrapper).toContainReactText(`Default: ${newDefaultOption}`);
Expand Down Expand Up @@ -881,7 +881,7 @@ describe('useBaseList', () => {
optionName: newDefaultOption,
optionValue: newDefaultOptionValue,
},
]);
] as any);
Copy link
Member

Choose a reason for hiding this comment

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

Having to cast this as any seems odd. How come it's needed? Is there a way we can change the signature of newDefaultPrice/newDefaultOption/newDefaultOptionValue to make it compliant?


expect(wrapper).toContainReactText(
`Default: ${newDefaultPrice}Default: ${newDefaultOption}Default: ${newDefaultOptionValue}`,
Expand Down
10 changes: 6 additions & 4 deletions packages/react-form/src/hooks/list/tests/dynamiclist.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ describe('useDynamicList', () => {

const sort1 = wrapper
.findAll('li')
.map((i) => i.find(TextField).props.value);
.map((i) => i.find(TextField)!.props.value);

expect(sort1).toStrictEqual(['A', 'C', 'B']);
});
Expand Down Expand Up @@ -235,7 +235,9 @@ describe('useDynamicList', () => {
});

it('handles dirty state when adding a field and resetting it', () => {
const wrapper = mount(<DynamicListComponent list={randomVariants()} />);
const wrapper = mount(
<DynamicListComponent list={randomVariants(1)} />,
);

expect(wrapper).toContainReactText('Dirty: false');

Expand Down Expand Up @@ -372,7 +374,7 @@ describe('useDynamicList', () => {
</>
))}

<button type="button" onClick={onNewDefault}>
<button type="button" onClick={onNewDefault as any}>
Copy link
Member

Choose a reason for hiding this comment

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

Having to cast this as any seems odd. How come it's needed? Is there a way we can change the signature of onNewDefault to make it compliant?

Default
</button>
<button type="button" onClick={() => addItem()}>
Expand Down Expand Up @@ -430,7 +432,7 @@ describe('useDynamicList', () => {
optionName: newDefaultOption,
optionValue: newDefaultOptionValue,
},
]);
] as any);
Copy link
Member

Choose a reason for hiding this comment

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

Having to cast this as any seems odd. How come it's needed? Is there a way we can change the signature of newDefaultPrice/newDefaultOption/newDefaultOptionValue to make it compliant?


expect(wrapper).toContainReactText(`Default: ${newDefaultPrice}`);
expect(wrapper).toContainReactText(`Default: ${newDefaultOption}`);
Expand Down
23 changes: 12 additions & 11 deletions packages/react-form/src/hooks/tests/form-with-dynamic-list.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import {mount, Root} from '@shopify/react-testing';
import {mount} from '@shopify/react-testing';

import {submitSuccess, submitFail} from '..';

Expand Down Expand Up @@ -30,7 +30,7 @@ describe('useForm with dynamic list', () => {
<FormWithDynamicVariantList data={fakeProduct()} />,
);

wrapper.find('button', {children: 'Add item'}).trigger('onClick');
wrapper.find('button', {children: 'Add item'})!.trigger('onClick');

expect(isDirty(wrapper)).toBe(true);
});
Expand All @@ -40,7 +40,7 @@ describe('useForm with dynamic list', () => {
<FormWithDynamicVariantList data={fakeProduct()} />,
);

wrapper.find('button', {children: 'Remove item'}).trigger('onClick');
wrapper.find('button', {children: 'Remove item'})!.trigger('onClick');

expect(isDirty(wrapper)).toBe(true);
});
Expand All @@ -51,7 +51,7 @@ describe('useForm with dynamic list', () => {
);

wrapper
.find(TextField, {label: 'price'})
.find(TextField, {label: 'price'})!
.trigger('onChange', 'next price');

expect(isDirty(wrapper)).toBe(true);
Expand Down Expand Up @@ -111,7 +111,7 @@ describe('useForm with dynamic list', () => {
<FormWithDynamicVariantList data={fakeProduct()} />,
);

wrapper.find('button', {children: 'Add item'}).trigger('onClick');
wrapper.find('button', {children: 'Add item'})!.trigger('onClick');

expect(wrapper).toContainReactComponentTimes(TextField, 3, {
label: 'option',
Expand All @@ -130,7 +130,7 @@ describe('useForm with dynamic list', () => {
<FormWithDynamicVariantList data={fakeProduct()} />,
);

wrapper.find('button', {children: 'Remove item'}).trigger('onClick');
wrapper.find('button', {children: 'Remove item'})!.trigger('onClick');

expect(wrapper).toContainReactComponentTimes(TextField, 1, {
label: 'option',
Expand All @@ -147,15 +147,16 @@ describe('useForm with dynamic list', () => {

describe('makeCleanAfterSubmit', () => {
it('sets dirty to false after successful submit if makeCleanAfterSubmit is true', async () => {
const promise = Promise.resolve(submitSuccess());
// eslint-disable-next-line promise/catch-or-return
Promise.resolve(submitSuccess());
const wrapper = mount(
<FormWithDynamicVariantList
data={fakeProduct()}
makeCleanAfterSubmit
/>,
);

wrapper.find('button', {children: 'Add item'}).trigger('onClick');
wrapper.find('button', {children: 'Add item'})!.trigger('onClick');
fillRequiredFields(wrapper);

expect(isDirty(wrapper)).toBe(true);
Expand All @@ -175,7 +176,7 @@ describe('useForm with dynamic list', () => {
/>,
);

wrapper.find('button', {children: 'Add item'}).trigger('onClick');
wrapper.find('button', {children: 'Add item'})!.trigger('onClick');
fillRequiredFields(wrapper);

expect(isDirty(wrapper)).toBe(true);
Expand All @@ -194,7 +195,7 @@ describe('useForm with dynamic list', () => {
/>,
);

wrapper.find('button', {children: 'Add item'}).trigger('onClick');
wrapper.find('button', {children: 'Add item'})!.trigger('onClick');
fillRequiredFields(wrapper);

expect(isDirty(wrapper)).toBe(true);
Expand All @@ -211,7 +212,7 @@ describe('useForm with dynamic list', () => {
<FormWithDynamicVariantList data={fakeProduct()} />,
);

wrapper.find('button', {children: 'Add item'}).trigger('onClick');
wrapper.find('button', {children: 'Add item'})!.trigger('onClick');
fillRequiredFields(wrapper);

expect(isDirty(wrapper)).toBe(true);
Expand Down
1 change: 0 additions & 1 deletion packages/react-form/src/hooks/tests/form.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,6 @@ describe('useForm', () => {
describe('makeClean', () => {
it(`cleans the form's dirty state`, () => {
const wrapper = mount(<ProductForm data={fakeProduct()} />);
const newProduct = 'Submarine full of gnomes';

changeTitle(wrapper, 'tortoritos, the chip for turtles!');

Expand Down
4 changes: 2 additions & 2 deletions packages/react-form/src/tests/utilities.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
} from '../utilities';
import {Field} from '../types';

function mockField<T>(params: Partial<Field<T>> | T): Field<T> {
function mockField<T>(params: Partial<Field<T>> | T): Field<T | undefined> {
const value = typeof params === 'string' ? params : undefined;
const fieldProps = typeof params === 'string' ? {} : params;

Expand Down Expand Up @@ -151,7 +151,7 @@ describe('reduceFields()', () => {
emails: [{work}],
};

const fieldsArray = reduceFields(
const fieldsArray = reduceFields<Field<any>[]>(
fieldBag,
(list, field) => list.concat(field),
[],
Expand Down
1 change: 0 additions & 1 deletion packages/react-form/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,5 @@
"./src/**/*.ts",
"./src/**/*.tsx"
],
"exclude": ["**/test/**/*", "**/tests/**/*"],
"references": [{"path": "../react-hooks"}, {"path": "../predicates"}]
}
Loading