diff --git a/.github/workflows/main-coverage.yml b/.github/workflows/main-coverage.yml new file mode 100644 index 00000000..8eb4854e --- /dev/null +++ b/.github/workflows/main-coverage.yml @@ -0,0 +1,38 @@ +name: Code Coverage (main) +on: + push: + branches: + - 'main' + +permissions: + contents: read + statuses: write + +jobs: + coverage: + name: Code Coverage + runs-on: ubuntu-latest + timeout-minutes: 3 + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Install pnpm + uses: pnpm/action-setup@v4 + + - name: Set up Node + uses: actions/setup-node@v4 + with: + cache: 'pnpm' + node-version-file: '.nvmrc' + + - name: Install dependencies + run: pnpm install + + - name: Run tests with coverage + run: pnpm run test:ci + + - name: Upload coverage report + uses: codecov/codecov-action@v5 + env: + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} diff --git a/docs/rules/no-node-access.md b/docs/rules/no-node-access.md index 7290cec3..4b79e961 100644 --- a/docs/rules/no-node-access.md +++ b/docs/rules/no-node-access.md @@ -4,11 +4,11 @@ -The Testing Library already provides methods for querying DOM elements. +Disallow direct access or manipulation of DOM nodes in favor of Testing Library's user-centric APIs. ## Rule Details -This rule aims to disallow DOM traversal using native HTML methods and properties, such as `closest`, `lastChild` and all that returns another Node element from an HTML tree. +This rule aims to disallow direct access and manipulation of DOM nodes using native HTML properties and methods — including traversal (e.g. `closest`, `lastChild`) as well as direct actions (e.g. `click()`, `focus()`). Use Testing Library’s queries and userEvent APIs instead. Examples of **incorrect** code for this rule: @@ -21,6 +21,12 @@ screen.getByText('Submit').closest('button'); // chaining with Testing Library m ```js import { screen } from '@testing-library/react'; +screen.getByText('Submit').click(); +``` + +```js +import { screen } from '@testing-library/react'; + const buttons = screen.getAllByRole('button'); expect(buttons[1].lastChild).toBeInTheDocument(); ``` @@ -41,6 +47,12 @@ const button = screen.getByRole('button'); expect(button).toHaveTextContent('submit'); ``` +```js +import { screen } from '@testing-library/react'; + +userEvent.click(screen.getByText('Submit')); +``` + ```js import { render, within } from '@testing-library/react'; diff --git a/lib/rules/no-node-access.ts b/lib/rules/no-node-access.ts index 14b7957d..d57e63ca 100644 --- a/lib/rules/no-node-access.ts +++ b/lib/rules/no-node-access.ts @@ -1,12 +1,21 @@ import { TSESTree, ASTUtils } from '@typescript-eslint/utils'; import { createTestingLibraryRule } from '../create-testing-library-rule'; -import { ALL_RETURNING_NODES } from '../utils'; +import { + ALL_RETURNING_NODES, + EVENT_HANDLER_METHODS, + EVENTS_SIMULATORS, +} from '../utils'; export const RULE_NAME = 'no-node-access'; export type MessageIds = 'noNodeAccess'; export type Options = [{ allowContainerFirstChild: boolean }]; +const ALL_PROHIBITED_MEMBERS = [ + ...ALL_RETURNING_NODES, + ...EVENT_HANDLER_METHODS, +] as const; + export default createTestingLibraryRule({ name: RULE_NAME, meta: { @@ -56,11 +65,15 @@ export default createTestingLibraryRule({ ? node.property.name : null; + const objectName = ASTUtils.isIdentifier(node.object) + ? node.object.name + : null; if ( propertyName && - ALL_RETURNING_NODES.some( + ALL_PROHIBITED_MEMBERS.some( (allReturningNode) => allReturningNode === propertyName - ) + ) && + !EVENTS_SIMULATORS.some((simulator) => simulator === objectName) ) { if (allowContainerFirstChild && propertyName === 'firstChild') { return; diff --git a/lib/utils/index.ts b/lib/utils/index.ts index e43b8102..aef73207 100644 --- a/lib/utils/index.ts +++ b/lib/utils/index.ts @@ -114,6 +114,14 @@ const METHODS_RETURNING_NODES = [ 'querySelectorAll', ] as const; +const EVENT_HANDLER_METHODS = [ + 'click', + 'focus', + 'blur', + 'select', + 'submit', +] as const; + const ALL_RETURNING_NODES = [ ...PROPERTIES_RETURNING_NODES, ...METHODS_RETURNING_NODES, @@ -147,4 +155,5 @@ export { ALL_RETURNING_NODES, PRESENCE_MATCHERS, ABSENCE_MATCHERS, + EVENT_HANDLER_METHODS, }; diff --git a/package.json b/package.json index 4ba0d923..639769de 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,7 @@ "types": "index.d.ts", "scripts": { "prebuild": "del-cli dist", - "build": "tsc", + "build": "tsc -p ./tsconfig.build.json", "generate-all": "pnpm run --parallel \"/^generate:.*/\"", "generate-all:check": "pnpm run generate-all && git diff --exit-code", "generate:configs": "ts-node tools/generate-configs", diff --git a/tests/index.test.ts b/tests/index.test.ts index 03cdbe2b..0d3ab20d 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -61,7 +61,7 @@ it('should export configs that refer to actual rules', () => { 'flat/marko', ]); const allConfigRules = Object.values(allConfigs) - .map((config) => Object.keys(config.rules)) + .map((config) => Object.keys(config.rules ?? {})) .reduce((previousValue, currentValue) => [ ...previousValue, ...currentValue, diff --git a/tests/lib/rules/no-node-access.test.ts b/tests/lib/rules/no-node-access.test.ts index cdecd5a9..fab437d4 100644 --- a/tests/lib/rules/no-node-access.test.ts +++ b/tests/lib/rules/no-node-access.test.ts @@ -1,11 +1,17 @@ -import { type TSESLint } from '@typescript-eslint/utils'; +import { InvalidTestCase, ValidTestCase } from '@typescript-eslint/rule-tester'; -import rule, { RULE_NAME, Options } from '../../../lib/rules/no-node-access'; +import rule, { + RULE_NAME, + Options, + MessageIds, +} from '../../../lib/rules/no-node-access'; +import { EVENT_HANDLER_METHODS, EVENTS_SIMULATORS } from '../../../lib/utils'; import { createRuleTester } from '../test-utils'; const ruleTester = createRuleTester(); -type ValidTestCase = TSESLint.ValidTestCase; +type RuleValidTestCase = ValidTestCase; +type RuleInvalidTestCase = InvalidTestCase; const SUPPORTED_TESTING_FRAMEWORKS = [ '@testing-library/angular', @@ -15,7 +21,7 @@ const SUPPORTED_TESTING_FRAMEWORKS = [ ]; ruleTester.run(RULE_NAME, rule, { - valid: SUPPORTED_TESTING_FRAMEWORKS.flatMap( + valid: SUPPORTED_TESTING_FRAMEWORKS.flatMap( (testingFramework) => [ { code: ` @@ -100,7 +106,7 @@ ruleTester.run(RULE_NAME, rule, { code: `/* related to issue #386 fix * now all node accessing properties (listed in lib/utils/index.ts, in PROPERTIES_RETURNING_NODES) * will not be reported by this rule because anything props.something won't be reported. - */ + */ import { screen } from '${testingFramework}'; function ComponentA(props) { if (props.firstChild) { @@ -142,21 +148,29 @@ ruleTester.run(RULE_NAME, rule, { // Example from discussions in issue #386 code: ` import { render } from '${testingFramework}'; - + function Wrapper({ children }) { // this should NOT be reported if (children) { // ... } - + // this should NOT be reported return
{children}
}; - + render(); expect(screen.getByText('SomeComponent')).toBeInTheDocument(); `, }, + ...EVENTS_SIMULATORS.map((simulator) => ({ + code: ` + import { screen } from '${testingFramework}'; + + const buttonText = screen.getByText('submit'); + ${simulator}.click(buttonText); + `, + })), ] ), invalid: SUPPORTED_TESTING_FRAMEWORKS.flatMap((testingFramework) => [ @@ -395,5 +409,24 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, + ...EVENT_HANDLER_METHODS.map((method) => ({ + code: ` + import { screen } from '${testingFramework}'; + + const button = document.getElementById('submit-btn').${method}(); + `, + errors: [ + { + line: 4, + column: 33, + messageId: 'noNodeAccess', + }, + { + line: 4, + column: 62, + messageId: 'noNodeAccess', + }, + ], + })), ]), }); diff --git a/tests/lib/rules/no-wait-for-side-effects.test.ts b/tests/lib/rules/no-wait-for-side-effects.test.ts index 2cdbe093..c7eed01a 100644 --- a/tests/lib/rules/no-wait-for-side-effects.test.ts +++ b/tests/lib/rules/no-wait-for-side-effects.test.ts @@ -1,4 +1,9 @@ -import rule, { RULE_NAME } from '../../../lib/rules/no-wait-for-side-effects'; +import { InvalidTestCase } from '@typescript-eslint/rule-tester'; + +import rule, { + RULE_NAME, + type MessageIds, +} from '../../../lib/rules/no-wait-for-side-effects'; import { createRuleTester } from '../test-utils'; const ruleTester = createRuleTester(); @@ -118,7 +123,7 @@ ruleTester.run(RULE_NAME, rule, { code: ` import { waitFor } from '${testingFramework}'; import { notUserEvent } from 'somewhere-else'; - + waitFor(() => { await notUserEvent.click(button) }) @@ -736,7 +741,7 @@ ruleTester.run(RULE_NAME, rule, { expect(b).toEqual('b') }).then(() => { userEvent.click(button) // Side effects are allowed inside .then() - expect(b).toEqual('b') + expect(b).toEqual('b') }) `, errors: [{ line: 4, column: 11, messageId: 'noSideEffectsWaitFor' }], @@ -808,9 +813,10 @@ ruleTester.run(RULE_NAME, rule, { } as const, ]), - ...SUPPORTED_TESTING_FRAMEWORKS.flatMap((testingFramework) => [ - { - code: ` + ...SUPPORTED_TESTING_FRAMEWORKS.flatMap>( + (testingFramework) => [ + { + code: ` import { waitFor } from '${testingFramework}'; import userEvent from '@testing-library/user-event' @@ -820,8 +826,9 @@ ruleTester.run(RULE_NAME, rule, { }); }); `, - errors: [{ line: 7, column: 13, messageId: 'noSideEffectsWaitFor' }], - }, - ]), + errors: [{ line: 7, column: 13, messageId: 'noSideEffectsWaitFor' }], + }, + ] + ), ], }); diff --git a/tests/lib/rules/prefer-find-by.test.ts b/tests/lib/rules/prefer-find-by.test.ts index c9d2c64b..b4728c15 100644 --- a/tests/lib/rules/prefer-find-by.test.ts +++ b/tests/lib/rules/prefer-find-by.test.ts @@ -191,7 +191,7 @@ ruleTester.run(RULE_NAME, rule, { }, ]), invalid: SUPPORTED_TESTING_FRAMEWORKS.flatMap((testingFramework) => [ - ...createScenario((waitMethod: string, queryMethod: string) => ({ + ...createScenario((waitMethod, queryMethod) => ({ code: ` import {${waitMethod}, screen} from '${testingFramework}'; it('tests', async () => { @@ -353,7 +353,7 @@ ruleTester.run(RULE_NAME, rule, { output: null, }, // presence matchers - ...createScenario((waitMethod: string, queryMethod: string) => ({ + ...createScenario((waitMethod, queryMethod) => ({ code: ` import {${waitMethod}} from '${testingFramework}'; it('tests', async () => { @@ -382,7 +382,7 @@ ruleTester.run(RULE_NAME, rule, { }) `, })), - ...createScenario((waitMethod: string, queryMethod: string) => ({ + ...createScenario((waitMethod, queryMethod) => ({ code: ` import {${waitMethod}} from '${testingFramework}'; it('tests', async () => { @@ -411,7 +411,7 @@ ruleTester.run(RULE_NAME, rule, { }) `, })), - ...createScenario((waitMethod: string, queryMethod: string) => ({ + ...createScenario((waitMethod, queryMethod) => ({ code: ` import {${waitMethod}} from '${testingFramework}'; it('tests', async () => { @@ -440,7 +440,7 @@ ruleTester.run(RULE_NAME, rule, { }) `, })), - ...createScenario((waitMethod: string, queryMethod: string) => ({ + ...createScenario((waitMethod, queryMethod) => ({ code: ` import {${waitMethod}} from '${testingFramework}'; it('tests', async () => { @@ -469,7 +469,7 @@ ruleTester.run(RULE_NAME, rule, { }) `, })), - ...createScenario((waitMethod: string, queryMethod: string) => ({ + ...createScenario((waitMethod, queryMethod) => ({ code: ` import {${waitMethod}} from '${testingFramework}'; it('tests', async () => { @@ -498,7 +498,7 @@ ruleTester.run(RULE_NAME, rule, { }) `, })), - ...createScenario((waitMethod: string, queryMethod: string) => ({ + ...createScenario((waitMethod, queryMethod) => ({ code: ` import {${waitMethod}} from '${testingFramework}'; it('tests', async () => { @@ -527,7 +527,7 @@ ruleTester.run(RULE_NAME, rule, { }) `, })), - ...createScenario((waitMethod: string, queryMethod: string) => ({ + ...createScenario((waitMethod, queryMethod) => ({ code: ` import {${waitMethod}} from '${testingFramework}'; it('tests', async () => { @@ -556,7 +556,7 @@ ruleTester.run(RULE_NAME, rule, { }) `, })), - ...createScenario((waitMethod: string, queryMethod: string) => ({ + ...createScenario((waitMethod, queryMethod) => ({ code: ` import {${waitMethod}} from '${testingFramework}'; it('tests', async () => { @@ -583,7 +583,7 @@ ruleTester.run(RULE_NAME, rule, { }) `, })), - ...createScenario((waitMethod: string, queryMethod: string) => ({ + ...createScenario((waitMethod, queryMethod) => ({ code: ` import {${waitMethod}} from '${testingFramework}'; it('tests', async () => { @@ -610,7 +610,7 @@ ruleTester.run(RULE_NAME, rule, { }) `, })), - ...createScenario((waitMethod: string, queryMethod: string) => ({ + ...createScenario((waitMethod, queryMethod) => ({ code: ` import {${waitMethod}} from '${testingFramework}'; it('tests', async () => { @@ -637,7 +637,7 @@ ruleTester.run(RULE_NAME, rule, { }) `, })), - ...createScenario((waitMethod: string, queryMethod: string) => ({ + ...createScenario((waitMethod, queryMethod) => ({ code: ` import {${waitMethod}} from '${testingFramework}'; it('tests', async () => { @@ -664,7 +664,7 @@ ruleTester.run(RULE_NAME, rule, { }) `, })), - ...createScenario((waitMethod: string, queryMethod: string) => ({ + ...createScenario((waitMethod, queryMethod) => ({ code: ` import {${waitMethod}} from '${testingFramework}'; it('tests', async () => { @@ -693,7 +693,7 @@ ruleTester.run(RULE_NAME, rule, { })), // Issue #579, https://github.com/testing-library/eslint-plugin-testing-library/issues/579 // findBy can have two sets of options: await screen.findByText('text', queryOptions, waitForOptions) - ...createScenario((waitMethod: string, queryMethod: string) => ({ + ...createScenario((waitMethod, queryMethod) => ({ code: `import {${waitMethod}} from '${testingFramework}'; const button = await ${waitMethod}(() => screen.${queryMethod}('Count is: 0'), { timeout: 100, interval: 200 }) `, @@ -714,8 +714,9 @@ ruleTester.run(RULE_NAME, rule, { )}('Count is: 0', { timeout: 100, interval: 200 }) `, })), - ...ASYNC_QUERIES_COMBINATIONS.map((queryMethod) => ({ - code: ` + ...ASYNC_QUERIES_COMBINATIONS.map>( + (queryMethod) => ({ + code: ` import {waitFor} from '${testingFramework}'; it('tests', async () => { await waitFor(async () => { @@ -724,24 +725,25 @@ ruleTester.run(RULE_NAME, rule, { }) }) `, - errors: [ - { - messageId: 'preferFindBy', - data: { - queryVariant: getFindByQueryVariant(queryMethod), - queryMethod: queryMethod.split('By')[1], - prevQuery: queryMethod, - waitForMethodName: 'waitFor', + errors: [ + { + messageId: 'preferFindBy', + data: { + queryVariant: getFindByQueryVariant(queryMethod), + queryMethod: queryMethod.split('By')[1], + prevQuery: queryMethod, + waitForMethodName: 'waitFor', + }, }, - }, - ], - output: ` + ], + output: ` import {waitFor} from '${testingFramework}'; it('tests', async () => { const button = await screen.${queryMethod}("button", { name: "Submit" }) expect(button).toBeInTheDocument() }) `, - })), + }) + ), ]), }); diff --git a/tsconfig.build.json b/tsconfig.build.json new file mode 100644 index 00000000..f46aafbc --- /dev/null +++ b/tsconfig.build.json @@ -0,0 +1,4 @@ +{ + "extends": "./tsconfig.json", + "exclude": ["./tests/**/*.ts"] +} diff --git a/tsconfig.json b/tsconfig.json index b4fb3559..c52787a0 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -16,5 +16,5 @@ "outDir": "dist", "sourceMap": false }, - "include": ["./lib/**/*.ts"] + "include": ["./lib/**/*.ts", "./tests/**/*.ts"] }