diff --git a/README.md b/README.md index 923bdb38..35ea5a78 100644 --- a/README.md +++ b/README.md @@ -125,33 +125,34 @@ To enable this configuration use the `extends` property in your ## Supported Rules -| Rule | Description | Configurations | Fixable | -| -------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------- | ----------------------------------------------------------------- | ------------------ | -| [await-async-query](docs/rules/await-async-query.md) | Enforce promises from async queries to be handled | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | -| [await-async-utils](docs/rules/await-async-utils.md) | Enforce async utils to be awaited properly | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | -| [await-fire-event](docs/rules/await-fire-event.md) | Enforce promises from fire event methods to be handled | ![vue-badge][] | | -| [consistent-data-testid](docs/rules/consistent-data-testid.md) | Ensure `data-testid` values match a provided regex. | | | -| [no-await-sync-events](docs/rules/no-await-sync-events.md) | Disallow unnecessary `await` for sync events | | | -| [no-await-sync-query](docs/rules/no-await-sync-query.md) | Disallow unnecessary `await` for sync queries | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | -| [no-container](docs/rules/no-container.md) | Disallow the use of `container` methods | ![angular-badge][] ![react-badge][] ![vue-badge][] | | -| [no-debug](docs/rules/no-debug.md) | Disallow the use of `debug` | ![angular-badge][] ![react-badge][] ![vue-badge][] | | -| [no-dom-import](docs/rules/no-dom-import.md) | Disallow importing from DOM Testing Library | ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] | -| [no-manual-cleanup](docs/rules/no-manual-cleanup.md) | Disallow the use of `cleanup` | | | -| [no-multiple-assertions-wait-for](docs/rules/no-multiple-assertions-wait-for.md) | Disallow the use of multiple expect inside `waitFor` | | | -| [no-node-access](docs/rules/no-node-access.md) | Disallow direct Node access | ![angular-badge][] ![react-badge][] ![vue-badge][] | | -| [no-promise-in-fire-event](docs/rules/no-promise-in-fire-event.md) | Disallow the use of promises passed to a `fireEvent` method | | | -| [no-render-in-setup](docs/rules/no-render-in-setup.md) | Disallow the use of `render` in setup functions | | | -| [no-side-effects-wait-for](docs/rules/no-side-effects-wait-for.md) | Disallow the use of side effects inside `waitFor` | | | -| [no-wait-for-empty-callback](docs/rules/no-wait-for-empty-callback.md) | Disallow empty callbacks for `waitFor` and `waitForElementToBeRemoved` | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | -| [no-wait-for-snapshot](docs/rules/no-wait-for-snapshot.md) | Ensures no snapshot is generated inside of a `waitFor` call | | | -| [no-wait-for-snapshot](docs/rules/no-wait-for-snapshot.md) | Ensures no snapshot is generated inside of a `waitFor` call | | | -| [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | | -| [prefer-find-by](docs/rules/prefer-find-by.md) | Suggest using `findBy*` methods instead of the `waitFor` + `getBy` queries | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] | -| [prefer-presence-queries](docs/rules/prefer-presence-queries.md) | Enforce specific queries when checking element is present or not | | | -| [prefer-user-event](docs/rules/prefer-user-event.md) | Suggest using `userEvent` library instead of `fireEvent` for simulating user interaction | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | -| [prefer-screen-queries](docs/rules/prefer-screen-queries.md) | Suggest using screen while using queries | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | -| [prefer-wait-for](docs/rules/prefer-wait-for.md) | Use `waitFor` instead of deprecated wait methods | | ![fixable-badge][] | -| [render-result-naming-convention](docs/rules/render-result-naming-convention.md) | Enforce a valid naming for return value from `render` | ![angular-badge][] ![react-badge][] ![vue-badge][] | | +| Rule | Description | Configurations | Fixable | +| -------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------- | ------------------ | +| [await-async-query](docs/rules/await-async-query.md) | Enforce promises from async queries to be handled | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | +| [await-async-utils](docs/rules/await-async-utils.md) | Enforce async utils to be awaited properly | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | +| [await-fire-event](docs/rules/await-fire-event.md) | Enforce promises from fire event methods to be handled | ![vue-badge][] | | +| [consistent-data-testid](docs/rules/consistent-data-testid.md) | Ensure `data-testid` values match a provided regex. | | | +| [no-await-sync-events](docs/rules/no-await-sync-events.md) | Disallow unnecessary `await` for sync events | | | +| [no-await-sync-query](docs/rules/no-await-sync-query.md) | Disallow unnecessary `await` for sync queries | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | +| [no-container](docs/rules/no-container.md) | Disallow the use of `container` methods | ![angular-badge][] ![react-badge][] ![vue-badge][] | | +| [no-debug](docs/rules/no-debug.md) | Disallow the use of `debug` | ![angular-badge][] ![react-badge][] ![vue-badge][] | | +| [no-dom-import](docs/rules/no-dom-import.md) | Disallow importing from DOM Testing Library | ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] | +| [no-manual-cleanup](docs/rules/no-manual-cleanup.md) | Disallow the use of `cleanup` | | | +| [no-multiple-assertions-wait-for](docs/rules/no-multiple-assertions-wait-for.md) | Disallow the use of multiple expect inside `waitFor` | | | +| [no-node-access](docs/rules/no-node-access.md) | Disallow direct Node access | ![angular-badge][] ![react-badge][] ![vue-badge][] | | +| [no-promise-in-fire-event](docs/rules/no-promise-in-fire-event.md) | Disallow the use of promises passed to a `fireEvent` method | | | +| [no-render-in-setup](docs/rules/no-render-in-setup.md) | Disallow the use of `render` in setup functions | | | +| [no-side-effects-wait-for](docs/rules/no-side-effects-wait-for.md) | Disallow the use of side effects inside `waitFor` | | | +| [no-unnecessary-act](docs/rules/no-unnecessary-act.md) | Disallow the use of `act` when wrapping Testing Library functions or functions with an empty body | | | +| [no-side-effects-wait-for](docs/rules/no-side-effects-wait-for.md) | Disallow the use of side effects inside `waitFor` | | | +| [no-wait-for-empty-callback](docs/rules/no-wait-for-empty-callback.md) | Disallow empty callbacks for `waitFor` and `waitForElementToBeRemoved` | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | | +| [no-wait-for-snapshot](docs/rules/no-wait-for-snapshot.md) | Ensures no snapshot is generated inside of a `waitFor` call | | | +| [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | | +| [prefer-find-by](docs/rules/prefer-find-by.md) | Suggest using `findBy*` methods instead of the `waitFor` + `getBy` queries | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] | +| [prefer-presence-queries](docs/rules/prefer-presence-queries.md) | Enforce specific queries when checking element is present or not | | | +| [prefer-user-event](docs/rules/prefer-user-event.md) | Suggest using `userEvent` library instead of `fireEvent` for simulating user interaction | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | +| [prefer-screen-queries](docs/rules/prefer-screen-queries.md) | Suggest using screen while using queries | ![dom-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | | +| [prefer-wait-for](docs/rules/prefer-wait-for.md) | Use `waitFor` instead of deprecated wait methods | | ![fixable-badge][] | +| [render-result-naming-convention](docs/rules/render-result-naming-convention.md) | Enforce a valid naming for return value from `render` | ![angular-badge][] ![react-badge][] ![vue-badge][] | | [build-badge]: https://github.com/testing-library/eslint-plugin-testing-library/actions/workflows/pipeline.yml/badge.svg [build-url]: https://github.com/testing-library/eslint-plugin-testing-library/actions/workflows/pipeline.yml diff --git a/docs/rules/no-unnecessary-act.md b/docs/rules/no-unnecessary-act.md new file mode 100644 index 00000000..18811e7d --- /dev/null +++ b/docs/rules/no-unnecessary-act.md @@ -0,0 +1,26 @@ +# Disallow the use of `act` when wrapping Testing Library functions or functions with an empty body + +## Rule Details + +This rule disallows the usage of `act` when using it to wrap Testing Library functions, or functions with an empty body, which suppresses valid warnings. For more information, see [https://kcd.im/react-act](https://kcd.im/react-act). + +Examples of **incorrect** code for this rule: + +```js +import { fireEvent, act } from '@testing-library/react'; + +it('Should have foo', () => { + act(() => fireEvent.click(el)); +}); +``` + +Examples of **correct** code for this rule: + +```js +import { act } from '@testing-library/react'; +it('Should have foo and bar', () => { + act(() => { + stuffThatDoesNotUseRTL(); + }); +}); +``` \ No newline at end of file diff --git a/lib/index.ts b/lib/index.ts index f722c9eb..f0280ae4 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -11,6 +11,7 @@ import noManualCleanup from './rules/no-manual-cleanup'; import noNodeAccess from './rules/no-node-access'; import noPromiseInFireEvent from './rules/no-promise-in-fire-event'; import noRenderInSetup from './rules/no-render-in-setup'; +import noUnnecessaryAct from './rules/no-unnecessary-act'; import noWaitForEmptyCallback from './rules/no-wait-for-empty-callback'; import noWaitForSnapshot from './rules/no-wait-for-snapshot'; import preferExplicitAssert from './rules/prefer-explicit-assert'; @@ -39,6 +40,7 @@ const rules = { 'no-promise-in-fire-event': noPromiseInFireEvent, 'no-render-in-setup': noRenderInSetup, 'no-side-effects-wait-for': noSideEffectsWaitFor, + 'no-unnecessary-act': noUnnecessaryAct, 'no-wait-for-empty-callback': noWaitForEmptyCallback, 'no-wait-for-snapshot': noWaitForSnapshot, 'prefer-explicit-assert': preferExplicitAssert, diff --git a/lib/rules/no-unnecessary-act.ts b/lib/rules/no-unnecessary-act.ts new file mode 100644 index 00000000..f1dbb88f --- /dev/null +++ b/lib/rules/no-unnecessary-act.ts @@ -0,0 +1,85 @@ +import { TSESTree, ASTUtils } from '@typescript-eslint/experimental-utils'; +import { + isBlockStatement, + isCallExpression, + isMemberExpression +} from '../node-utils'; +import { createTestingLibraryRule } from '../create-testing-library-rule'; + +export const RULE_NAME = 'no-unnecessary-act'; +export type MessageIds = 'noUnnecessaryAct'; +type Options = []; + +const ACT_EXPRESSION_QUERY = 'CallExpression[callee.name=/^(act)$/]'; + +export default createTestingLibraryRule({ + name: RULE_NAME, + meta: { + type: 'problem', + docs: { + description: + 'Disallow the use of `act` when wrapping Testing Library functions or functions with an empty body', + category: 'Best Practices', + recommended: false + }, + messages: { + noUnnecessaryAct: + 'Avoid wrapping Testing Library functions in `act` as they are wrapped in act automatically, and avoid wrapping noop functions in `act` as they suppress valid warnings.' + }, + fixable: null, + schema: [] + }, + defaultOptions: [], + + create(context, _, helpers) { + function reportIfUnnecessary( + node: TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression + ) { + let isEmpty = false; + let callsTL = false; + const callsOnlyTLFns = (body: Array): boolean => + body.every((node: TSESTree.ExpressionStatement) => { + if ( + isCallExpression(node.expression) && + isMemberExpression(node.expression.callee) && + isCallExpression(node.expression.callee.object) && + helpers.isNodeComingFromTestingLibrary(node.expression.callee) + ) { + return true; + } + return false; + }); + if ( + isBlockStatement(node.body) && + node.body.body.length === 0 && + isCallExpression(node.parent) && + ASTUtils.isIdentifier(node.parent.callee) + ) { + isEmpty = true; + } + if ( + isCallExpression(node.body) && + isMemberExpression(node.body.callee) && + helpers.isNodeComingFromTestingLibrary(node.body.callee) + ) { + callsTL = true; + } + if ( + isEmpty || + callsTL || + (isBlockStatement(node.body) && callsOnlyTLFns(node.body.body)) + ) { + context.report({ + node, + loc: node.body.loc.start, + messageId: 'noUnnecessaryAct' + }); + } + } + + return { + [`${ACT_EXPRESSION_QUERY} > ArrowFunctionExpression`]: reportIfUnnecessary, + [`${ACT_EXPRESSION_QUERY} > FunctionExpression`]: reportIfUnnecessary + }; + } +}); diff --git a/tests/lib/rules/no-unnecessary-act.test.ts b/tests/lib/rules/no-unnecessary-act.test.ts new file mode 100644 index 00000000..8f285539 --- /dev/null +++ b/tests/lib/rules/no-unnecessary-act.test.ts @@ -0,0 +1,185 @@ +import { createRuleTester } from '../test-utils'; +import rule, { RULE_NAME } from '../../../lib/rules/no-unnecessary-act'; + +const ruleTester = createRuleTester({ + ecmaFeatures: { + jsx: true + } +}); + +ruleTester.run(RULE_NAME, rule, { + valid: [ + { + code: ` + import { act } from '@testing-library/react'; + act(() => { + stuffThatDoesNotUseRTL(); + }); + ` + }, + { + code: ` + const { act } = require('@testing-library/react'); + act(() => { + stuffThatDoesNotUseRTL(); + }); + ` + }, + { + code: ` + import ReactTestUtils from 'react-dom/test-utils' + ReactTestUtils.act(() => { + stuffThatDoesNotUseRTL(); + }); + ` + }, + { + code: ` + import { act, screen } from '@testing-library/react'; + act(() => { + screen.getByText('hello'); + stuffThatDoesNotUseRTL(); + }); + ` + }, + { + code: ` + import * as rtl from '@testing-library/react'; + rtl.act(() => { + rtl.fireEvent.click(el); + stuffThatDoesNotUseRTL(); + }); + ` + } + ], + invalid: [ + { + code: ` + import { act } from '@testing-library/react'; + await act(async () => {}); + `, + errors: [ + { + messageId: 'noUnnecessaryAct' + } + ] + }, + { + code: ` + import { act } from '@testing-library/react'; + act(() => {}); + `, + errors: [ + { + messageId: 'noUnnecessaryAct' + } + ] + }, + { + code: ` + import { act } from '@testing-library/react'; + import userEvent from '@testing-library/user-event'; + await act(async () => userEvent.type('hi', el)); + `, + errors: [ + { + messageId: 'noUnnecessaryAct' + } + ] + }, + { + code: ` + import { screen, act } from '@testing-library/react'; + act(() => screen.getByText('blah')); + `, + errors: [ + { + messageId: 'noUnnecessaryAct' + } + ] + }, + { + code: ` + import { fireEvent, act } from '@testing-library/react'; + act(() => fireEvent.click(el)); + `, + errors: [ + { + messageId: 'noUnnecessaryAct' + } + ] + }, + { + code: ` + import { fireEvent, act } from '@testing-library/react'; + act(() => { + fireEvent.click(el) + }); + `, + errors: [ + { + messageId: 'noUnnecessaryAct' + } + ] + }, + { + code: ` + import { act } from '@testing-library/react'; + import userEvent from '@testing-library/user-event'; + act(() => userEvent.click(el)); + `, + errors: [ + { + messageId: 'noUnnecessaryAct' + } + ] + }, + { + code: ` + import * as rtl from '@testing-library/react'; + import userEvent from '@testing-library/user-event'; + rtl.act(() => { + userEvent.click(el) + }); + `, + errors: [ + { + messageId: 'noUnnecessaryAct' + } + ] + }, + { + code: ` + import { act, render } from '@testing-library/react'; + act(() => render(
)); + `, + errors: [ + { + messageId: 'noUnnecessaryAct' + } + ] + }, + { + code: ` + import { act, render } from '@testing-library/react'; + await act(async () => render(
)); + `, + errors: [ + { + messageId: 'noUnnecessaryAct' + } + ] + }, + { + code: ` + import * as rtl from '@testing-library/react'; + rtl.act(() => rtl.screen.getByText('blah')); + `, + errors: [ + { + messageId: 'noUnnecessaryAct' + } + ] + } + ] +});