From cb2ab8e6726d9dab477c52169cdfa0a16b21d4c5 Mon Sep 17 00:00:00 2001 From: Hasegawa-Yukihiro Date: Fri, 6 Jun 2025 22:16:26 +0900 Subject: [PATCH 1/3] feat: add EVENT_HANDLER_METHODS to no-node-access rule --- lib/rules/no-node-access.ts | 9 +++++++-- lib/utils/index.ts | 9 +++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/rules/no-node-access.ts b/lib/rules/no-node-access.ts index 14b7957d..54b59736 100644 --- a/lib/rules/no-node-access.ts +++ b/lib/rules/no-node-access.ts @@ -1,12 +1,17 @@ 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 } 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: { @@ -58,7 +63,7 @@ export default createTestingLibraryRule({ if ( propertyName && - ALL_RETURNING_NODES.some( + ALL_PROHIBITED_MEMBERS.some( (allReturningNode) => allReturningNode === propertyName ) ) { 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, }; From 47f1ac9e82af3febb55eab708b0580ffe9616a5c Mon Sep 17 00:00:00 2001 From: Hasegawa-Yukihiro Date: Fri, 6 Jun 2025 22:17:18 +0900 Subject: [PATCH 2/3] test: add test --- tests/lib/rules/no-node-access.test.ts | 41 +++++++++++++++++++++----- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/tests/lib/rules/no-node-access.test.ts b/tests/lib/rules/no-node-access.test.ts index cdecd5a9..1d2112f0 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 } 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,17 +148,17 @@ 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(); `, @@ -395,5 +401,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', + }, + ], + })), ]), }); From 5476027659f658d31a23bea420ff5c9ec4eb4ec9 Mon Sep 17 00:00:00 2001 From: Hasegawa-Yukihiro Date: Fri, 6 Jun 2025 22:19:42 +0900 Subject: [PATCH 3/3] docs: update no-node-access rule details and examples --- docs/rules/no-node-access.md | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) 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';