Skip to content
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

test: setup TypeScript settings #824

Open
wants to merge 5 commits into
base: alpha
Choose a base branch
from
Open

test: setup TypeScript settings #824

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d05136a
test: setup TypeScript settings
koba04 May 10, 2022
168d733
refactor: migrate util.js to TypeScript
koba04 May 11, 2022
ab60306
refactor: migrate SSR-test.js to TypeScript
koba04 May 11, 2022
a1288b5
refactor: prettier
koba04 May 11, 2022
ac171df
refactor: migrate ChildMapping-test.js to TypeScript
koba04 May 11, 2022
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
7 changes: 4 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,12 @@
},
"homepage": "https://github.com/reactjs/react-transition-group#readme",
"jest": {
"testRegex": "-test\\.js",
"testRegex": "-test\\.[jt]sx?",
"setupFiles": [
"./test/setup.js"
"./test/setup.ts"
],
"setupFilesAfterEnv": [
"./test/setupAfterEnv.js"
"./test/setupAfterEnv.ts"
],
"roots": [
"<rootDir>/test"
Expand Down Expand Up @@ -83,6 +83,7 @@
"@storybook/addon-actions": "^6.3.4",
"@storybook/react": "^6.3.4",
"@testing-library/react": "alpha",
"@types/jest": "^27.5.0",
"@types/prop-types": "^15.7.4",
"@types/react": "^17.0.41",
"@types/react-dom": "^17.0.14",
Expand Down
8 changes: 4 additions & 4 deletions src/utils/ChildMapping.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ export function getChildMapping(
* in `next` in a reasonable order.
*/
export function mergeChildMappings(
prev: Record<string, ReactChild>,
next: Record<string, ReactChild>
prev_: Record<string, ReactChild> | undefined,
next_: Record<string, ReactChild> | undefined
) {
prev = prev || {};
next = next || {};
const prev = prev_ || {};
const next = next_ || {};

function getValueForKey(key: string) {
return key in next ? next[key] : prev[key];
Expand Down
1 change: 0 additions & 1 deletion test/.eslintrc.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
env:
jest: true
es6: true
rules:
no-require: off
global-require: off
Expand Down
113 changes: 54 additions & 59 deletions test/ChildMapping-test.js → test/ChildMapping-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
let React;
let ChildMapping;
import React from 'react';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not really a fan of TypeScript in runtime tests precisely because runtime tests are annoying to type. The previous test did work differently by clearing modules for each test. A static import is not the same.

If you want to test the types, we should have separate test files for them.

Copy link
Member Author

Choose a reason for hiding this comment

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

The previous test did work differently by clearing modules for each test. A static import is not the same.

Yeah, but this seems not to be necessary because the tests don't mock anything or mutate modules. I prefer not to mock or mutate modules as possible. So it would be nice to use static imports as default.

If you want to test the types, we should have separate test files for them.

That is a valid opinion, but I'd like to write tests in TypeScript. If typing test code is annoying, we can use any or type casting in that places.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I prefer not to mock or mutate modules as possible. So it would be nice to use static imports as default.

We're doing neither here. We're resetting the module which is not yet supported with Jest and ESM. So until then, we need to re-require.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you, I have two questions.

  • Why do we need to reset the module even though we haven't used mock or mutating modules?
  • Can't we reset modules with Jest even if we transpile ESM to CJS by tsc?

import * as ChildMapping from '../src/utils/ChildMapping';

describe('ChildMapping', () => {
beforeEach(() => {
React = require('react');
ChildMapping = require('../src/utils/ChildMapping');
});

it('should support getChildMapping', () => {
let oneone = <div key="oneone" />;
let onetwo = <div key="onetwo" />;
Expand All @@ -32,102 +27,102 @@ describe('ChildMapping', () => {

it('should support mergeChildMappings for adding keys', () => {
let prev = {
one: true,
two: true,
one: 'one',
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks like runtime behavior changed. But the PR title says it's only about test. Could you clarify why this change here is necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

The type is ReactChild, which doesn't accept boolean, so I've changed the test data to String.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure but that's not a reason to change runtime behavior, no? So far this looks like a breaking change just so that our tests are type-checked?

Copy link
Member Author

Choose a reason for hiding this comment

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

What does "runtime behavior" mean? This doesn't change any runtime behavior. The TypeScript migration causes this change. Do you mean that we shouldn't do any incompatible changes with DefinitelyTyped?

two: 'two',
};
let next = {
one: true,
two: true,
three: true,
one: 'one',
two: 'two',
three: 'three',
};
expect(ChildMapping.mergeChildMappings(prev, next)).toEqual({
one: true,
two: true,
three: true,
one: 'one',
two: 'two',
three: 'three',
});
});

it('should support mergeChildMappings for removing keys', () => {
let prev = {
one: true,
two: true,
three: true,
one: 'one',
two: 'two',
three: 'three',
};
let next = {
one: true,
two: true,
one: 'one',
two: 'two',
};
expect(ChildMapping.mergeChildMappings(prev, next)).toEqual({
one: true,
two: true,
three: true,
one: 'one',
two: 'two',
three: 'three',
});
});

it('should support mergeChildMappings for adding and removing', () => {
let prev = {
one: true,
two: true,
three: true,
one: 'one',
two: 'two',
three: 'three',
};
let next = {
one: true,
two: true,
four: true,
one: 'one',
two: 'two',
four: 'four',
};
expect(ChildMapping.mergeChildMappings(prev, next)).toEqual({
one: true,
two: true,
three: true,
four: true,
one: 'one',
two: 'two',
three: 'three',
four: 'four',
});
});

it('should reconcile overlapping insertions and deletions', () => {
let prev = {
one: true,
two: true,
four: true,
five: true,
one: 'one',
two: 'two',
four: 'four',
five: 'five',
};
let next = {
one: true,
two: true,
three: true,
five: true,
one: 'one',
two: 'two',
three: 'three',
five: 'five',
};
expect(ChildMapping.mergeChildMappings(prev, next)).toEqual({
one: true,
two: true,
three: true,
four: true,
five: true,
one: 'one',
two: 'two',
three: 'three',
four: 'four',
five: 'five',
});
});

it('should support mergeChildMappings with undefined input', () => {
let prev = {
one: true,
two: true,
const prev = {
one: 'one',
two: 'two',
};

let next;
const next = undefined;

expect(ChildMapping.mergeChildMappings(prev, next)).toEqual({
one: true,
two: true,
one: 'one',
two: 'two',
});

prev = undefined;
const prev2 = undefined;

next = {
three: true,
four: true,
const next2 = {
three: 'three',
four: 'four',
};

expect(ChildMapping.mergeChildMappings(prev, next)).toEqual({
three: true,
four: true,
expect(ChildMapping.mergeChildMappings(prev2, next2)).toEqual({
three: 'three',
four: 'four',
});
});
});
4 changes: 3 additions & 1 deletion test/SSR-test.js → test/SSR-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,7 @@
import * as ReactTransitionGroup from '../src'; // eslint-disable-line no-unused-vars

describe('SSR', () => {
it('should import react-transition-group in node env', () => {});
it('should import react-transition-group in node env', () => {
/* noop */
});
});
1 change: 1 addition & 0 deletions test/setup.js → test/setup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// @ts-ignore
global.requestAnimationFrame = function (callback) {
setTimeout(callback, 0);
};
0 test/setupAfterEnv.js → test/setupAfterEnv.ts
View file
Open in desktop
File renamed without changes.
9 changes: 6 additions & 3 deletions test/utils.js → test/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
import { render as baseRender } from '@testing-library/react/pure';
import {
render as baseRender,
RenderOptions,
} from '@testing-library/react/pure';
import React from 'react';

export * from '@testing-library/react';
export function render(element, options) {
export function render(element: React.ReactElement, options?: RenderOptions) {
const result = baseRender(element, options);

return {
...result,
setProps(props) {
setProps(props: any) {
result.rerender(React.cloneElement(element, props));
},
};
Expand Down
40 changes: 39 additions & 1 deletion yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3841,6 +3841,14 @@
"@types/istanbul-lib-coverage" "*"
"@types/istanbul-lib-report" "*"

"@types/jest@^27.5.0":
version "27.5.0"
resolved "https://registry.yarnpkg.com/@types/jest/-/jest-27.5.0.tgz#e04ed1824ca6b1dd0438997ba60f99a7405d4c7b"
integrity sha512-9RBFx7r4k+msyj/arpfaa0WOOEcaAZNmN+j80KFbFCoSqCJGHTz7YMAMGQW9Xmqm5w6l5c25vbSjMwlikJi5+g==
dependencies:
jest-matcher-utils "^27.0.0"
pretty-format "^27.0.0"

"@types/json-schema@^7.0.4", "@types/json-schema@^7.0.5", "@types/json-schema@^7.0.6", "@types/json-schema@^7.0.7":
version "7.0.7"
resolved "https://registry.yarnpkg.com/@types/json-schema/-/json-schema-7.0.7.tgz#98a993516c859eb0d5c4c8f098317a9ea68db9ad"
Expand Down Expand Up @@ -7299,6 +7307,11 @@ diff-sequences@^25.2.6:
resolved "https://registry.yarnpkg.com/diff-sequences/-/diff-sequences-25.2.6.tgz#5f467c00edd35352b7bca46d7927d60e687a76dd"
integrity sha512-Hq8o7+6GaZeoFjtpgvRBUknSXNeJiCx7V9Fr94ZMljNiCr9n9L8H8aJqgWOQiDDGdyn29fRNcDdRVJ5fdyihfg==

diff-sequences@^27.5.1:
version "27.5.1"
resolved "https://registry.yarnpkg.com/diff-sequences/-/diff-sequences-27.5.1.tgz#eaecc0d327fd68c8d9672a1e64ab8dccb2ef5327"
integrity sha512-k1gCAXAsNgLwEL+Y8Wvl+M6oEFj5bgazfZULpS5CneoPPXRaCCW7dm+q21Ky2VEE5X+VeRDBVg1Pcvvsr4TtNQ==

diffie-hellman@^5.0.0:
version "5.0.3"
resolved "https://registry.yarnpkg.com/diffie-hellman/-/diffie-hellman-5.0.3.tgz#40e8ee98f55a2149607146921c63e1ae5f3d2875"
Expand Down Expand Up @@ -10662,6 +10675,16 @@ jest-diff@^25.1.0, jest-diff@^25.3.0:
jest-get-type "^25.2.6"
pretty-format "^25.3.0"

jest-diff@^27.5.1:
version "27.5.1"
resolved "https://registry.yarnpkg.com/jest-diff/-/jest-diff-27.5.1.tgz#a07f5011ac9e6643cf8a95a462b7b1ecf6680def"
integrity sha512-m0NvkX55LDt9T4mctTEgnZk3fmEg3NRYutvMPWM/0iPnkFj2wIeF45O1718cMSOFO1vINkqmxqD8vE37uTEbqw==
dependencies:
chalk "^4.0.0"
diff-sequences "^27.5.1"
jest-get-type "^27.5.1"
pretty-format "^27.5.1"

jest-docblock@^25.3.0:
version "25.3.0"
resolved "https://registry.yarnpkg.com/jest-docblock/-/jest-docblock-25.3.0.tgz#8b777a27e3477cd77a168c05290c471a575623ef"
Expand Down Expand Up @@ -10709,6 +10732,11 @@ jest-get-type@^25.2.6:
resolved "https://registry.yarnpkg.com/jest-get-type/-/jest-get-type-25.2.6.tgz#0b0a32fab8908b44d508be81681487dbabb8d877"
integrity sha512-DxjtyzOHjObRM+sM1knti6or+eOgcGU4xVSb2HNP1TqO4ahsT+rqZg+nyqHWJSvWgKC5cG3QjGFBqxLghiF/Ig==

jest-get-type@^27.5.1:
version "27.5.1"
resolved "https://registry.yarnpkg.com/jest-get-type/-/jest-get-type-27.5.1.tgz#3cd613c507b0f7ace013df407a1c1cd578bcb4f1"
integrity sha512-2KY95ksYSaK7DMBWQn6dQz3kqAf3BB64y2udeG+hv4KfSOb9qwcYQstTJc1KCbsix+wLZWZYN8t7nwX3GOBLRw==

jest-haste-map@^25.3.0:
version "25.3.0"
resolved "https://registry.yarnpkg.com/jest-haste-map/-/jest-haste-map-25.3.0.tgz#b7683031c9c9ddc0521d311564108b244b11e4c6"
Expand Down Expand Up @@ -10769,6 +10797,16 @@ jest-matcher-utils@^25.3.0:
jest-get-type "^25.2.6"
pretty-format "^25.3.0"

jest-matcher-utils@^27.0.0:
version "27.5.1"
resolved "https://registry.yarnpkg.com/jest-matcher-utils/-/jest-matcher-utils-27.5.1.tgz#9c0cdbda8245bc22d2331729d1091308b40cf8ab"
integrity sha512-z2uTx/T6LBaCoNWNFWwChLBKYxTMcGBRjAt+2SbP929/Fflb9aa5LGma654Rz8z9HLxsrUaYzxE9T/EFIL/PAw==
dependencies:
chalk "^4.0.0"
jest-diff "^27.5.1"
jest-get-type "^27.5.1"
pretty-format "^27.5.1"

jest-message-util@^25.3.0:
version "25.3.0"
resolved "https://registry.yarnpkg.com/jest-message-util/-/jest-message-util-25.3.0.tgz#e3836826fe5ca538a337b87d9bd2648190867f85"
Expand Down Expand Up @@ -14219,7 +14257,7 @@ pretty-format@^25.3.0:
ansi-styles "^4.0.0"
react-is "^16.12.0"

pretty-format@^27.0.2:
pretty-format@^27.0.0, pretty-format@^27.0.2, pretty-format@^27.5.1:
version "27.5.1"
resolved "https://registry.yarnpkg.com/pretty-format/-/pretty-format-27.5.1.tgz#2181879fdea51a7a5851fb39d920faa63f01d88e"
integrity sha512-Qb1gy5OrP5+zDf2Bvnzdl3jsTf1qXVMazbvCoKhtKqVs4/YK4ozX4gKQJJVyNe+cajNPn0KoC0MC3FUmaHWEmQ==
Expand Down