Skip to content

Commit 934bc5e

Browse files
authored
feat(no-node-access): disallow DOM event methods (#1023)
Closes #752
1 parent 665577c commit 934bc5e

File tree

4 files changed

+80
-13
lines changed

4 files changed

+80
-13
lines changed

docs/rules/no-node-access.md

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@
44

55
<!-- end auto-generated rule header -->
66

7-
The Testing Library already provides methods for querying DOM elements.
7+
Disallow direct access or manipulation of DOM nodes in favor of Testing Library's user-centric APIs.
88

99
## Rule Details
1010

11-
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.
11+
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.
1212

1313
Examples of **incorrect** code for this rule:
1414

@@ -21,6 +21,12 @@ screen.getByText('Submit').closest('button'); // chaining with Testing Library m
2121
```js
2222
import { screen } from '@testing-library/react';
2323

24+
screen.getByText('Submit').click();
25+
```
26+
27+
```js
28+
import { screen } from '@testing-library/react';
29+
2430
const buttons = screen.getAllByRole('button');
2531
expect(buttons[1].lastChild).toBeInTheDocument();
2632
```
@@ -41,6 +47,12 @@ const button = screen.getByRole('button');
4147
expect(button).toHaveTextContent('submit');
4248
```
4349

50+
```js
51+
import { screen } from '@testing-library/react';
52+
53+
userEvent.click(screen.getByText('Submit'));
54+
```
55+
4456
```js
4557
import { render, within } from '@testing-library/react';
4658

lib/rules/no-node-access.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,21 @@
11
import { TSESTree, ASTUtils } from '@typescript-eslint/utils';
22

33
import { createTestingLibraryRule } from '../create-testing-library-rule';
4-
import { ALL_RETURNING_NODES } from '../utils';
4+
import {
5+
ALL_RETURNING_NODES,
6+
EVENT_HANDLER_METHODS,
7+
EVENTS_SIMULATORS,
8+
} from '../utils';
59

610
export const RULE_NAME = 'no-node-access';
711
export type MessageIds = 'noNodeAccess';
812
export type Options = [{ allowContainerFirstChild: boolean }];
913

14+
const ALL_PROHIBITED_MEMBERS = [
15+
...ALL_RETURNING_NODES,
16+
...EVENT_HANDLER_METHODS,
17+
] as const;
18+
1019
export default createTestingLibraryRule<Options, MessageIds>({
1120
name: RULE_NAME,
1221
meta: {
@@ -56,11 +65,15 @@ export default createTestingLibraryRule<Options, MessageIds>({
5665
? node.property.name
5766
: null;
5867

68+
const objectName = ASTUtils.isIdentifier(node.object)
69+
? node.object.name
70+
: null;
5971
if (
6072
propertyName &&
61-
ALL_RETURNING_NODES.some(
73+
ALL_PROHIBITED_MEMBERS.some(
6274
(allReturningNode) => allReturningNode === propertyName
63-
)
75+
) &&
76+
!EVENTS_SIMULATORS.some((simulator) => simulator === objectName)
6477
) {
6578
if (allowContainerFirstChild && propertyName === 'firstChild') {
6679
return;

lib/utils/index.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,14 @@ const METHODS_RETURNING_NODES = [
114114
'querySelectorAll',
115115
] as const;
116116

117+
const EVENT_HANDLER_METHODS = [
118+
'click',
119+
'focus',
120+
'blur',
121+
'select',
122+
'submit',
123+
] as const;
124+
117125
const ALL_RETURNING_NODES = [
118126
...PROPERTIES_RETURNING_NODES,
119127
...METHODS_RETURNING_NODES,
@@ -147,4 +155,5 @@ export {
147155
ALL_RETURNING_NODES,
148156
PRESENCE_MATCHERS,
149157
ABSENCE_MATCHERS,
158+
EVENT_HANDLER_METHODS,
150159
};

tests/lib/rules/no-node-access.test.ts

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
1-
import { type TSESLint } from '@typescript-eslint/utils';
1+
import { InvalidTestCase, ValidTestCase } from '@typescript-eslint/rule-tester';
22

3-
import rule, { RULE_NAME, Options } from '../../../lib/rules/no-node-access';
3+
import rule, {
4+
RULE_NAME,
5+
Options,
6+
MessageIds,
7+
} from '../../../lib/rules/no-node-access';
8+
import { EVENT_HANDLER_METHODS, EVENTS_SIMULATORS } from '../../../lib/utils';
49
import { createRuleTester } from '../test-utils';
510

611
const ruleTester = createRuleTester();
712

8-
type ValidTestCase = TSESLint.ValidTestCase<Options>;
13+
type RuleValidTestCase = ValidTestCase<Options>;
14+
type RuleInvalidTestCase = InvalidTestCase<MessageIds, Options>;
915

1016
const SUPPORTED_TESTING_FRAMEWORKS = [
1117
'@testing-library/angular',
@@ -15,7 +21,7 @@ const SUPPORTED_TESTING_FRAMEWORKS = [
1521
];
1622

1723
ruleTester.run(RULE_NAME, rule, {
18-
valid: SUPPORTED_TESTING_FRAMEWORKS.flatMap<ValidTestCase>(
24+
valid: SUPPORTED_TESTING_FRAMEWORKS.flatMap<RuleValidTestCase>(
1925
(testingFramework) => [
2026
{
2127
code: `
@@ -100,7 +106,7 @@ ruleTester.run(RULE_NAME, rule, {
100106
code: `/* related to issue #386 fix
101107
* now all node accessing properties (listed in lib/utils/index.ts, in PROPERTIES_RETURNING_NODES)
102108
* will not be reported by this rule because anything props.something won't be reported.
103-
*/
109+
*/
104110
import { screen } from '${testingFramework}';
105111
function ComponentA(props) {
106112
if (props.firstChild) {
@@ -142,21 +148,29 @@ ruleTester.run(RULE_NAME, rule, {
142148
// Example from discussions in issue #386
143149
code: `
144150
import { render } from '${testingFramework}';
145-
151+
146152
function Wrapper({ children }) {
147153
// this should NOT be reported
148154
if (children) {
149155
// ...
150156
}
151-
157+
152158
// this should NOT be reported
153159
return <div className="wrapper-class">{children}</div>
154160
};
155-
161+
156162
render(<Wrapper><SomeComponent /></Wrapper>);
157163
expect(screen.getByText('SomeComponent')).toBeInTheDocument();
158164
`,
159165
},
166+
...EVENTS_SIMULATORS.map((simulator) => ({
167+
code: `
168+
import { screen } from '${testingFramework}';
169+
170+
const buttonText = screen.getByText('submit');
171+
${simulator}.click(buttonText);
172+
`,
173+
})),
160174
]
161175
),
162176
invalid: SUPPORTED_TESTING_FRAMEWORKS.flatMap((testingFramework) => [
@@ -395,5 +409,24 @@ ruleTester.run(RULE_NAME, rule, {
395409
},
396410
],
397411
},
412+
...EVENT_HANDLER_METHODS.map<RuleInvalidTestCase>((method) => ({
413+
code: `
414+
import { screen } from '${testingFramework}';
415+
416+
const button = document.getElementById('submit-btn').${method}();
417+
`,
418+
errors: [
419+
{
420+
line: 4,
421+
column: 33,
422+
messageId: 'noNodeAccess',
423+
},
424+
{
425+
line: 4,
426+
column: 62,
427+
messageId: 'noNodeAccess',
428+
},
429+
],
430+
})),
398431
]),
399432
});

0 commit comments

Comments
 (0)