Skip to content

feat(no-node-access): extend no-node-access to disallow DOM event methods #1023

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.

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 14 additions & 2 deletions docs/rules/no-node-access.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@

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

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:

Expand All @@ -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();
```
Expand All @@ -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';

Expand Down
9 changes: 7 additions & 2 deletions lib/rules/no-node-access.ts
Original file line number Diff line number Diff line change
@@ -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<Options, MessageIds>({
name: RULE_NAME,
meta: {
Expand Down Expand Up @@ -58,7 +63,7 @@ export default createTestingLibraryRule<Options, MessageIds>({

if (
propertyName &&
ALL_RETURNING_NODES.some(
ALL_PROHIBITED_MEMBERS.some(
(allReturningNode) => allReturningNode === propertyName
)
) {
Expand Down
9 changes: 9 additions & 0 deletions lib/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -147,4 +155,5 @@ export {
ALL_RETURNING_NODES,
PRESENCE_MATCHERS,
ABSENCE_MATCHERS,
EVENT_HANDLER_METHODS,
};
41 changes: 33 additions & 8 deletions tests/lib/rules/no-node-access.test.ts
Original file line number Diff line number Diff line change
@@ -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<Options>;
type RuleValidTestCase = ValidTestCase<Options>;
type RuleInvalidTestCase = InvalidTestCase<MessageIds, Options>;

const SUPPORTED_TESTING_FRAMEWORKS = [
'@testing-library/angular',
Expand All @@ -15,7 +21,7 @@ const SUPPORTED_TESTING_FRAMEWORKS = [
];

ruleTester.run(RULE_NAME, rule, {
valid: SUPPORTED_TESTING_FRAMEWORKS.flatMap<ValidTestCase>(
valid: SUPPORTED_TESTING_FRAMEWORKS.flatMap<RuleValidTestCase>(
(testingFramework) => [
{
code: `
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 <div className="wrapper-class">{children}</div>
};

render(<Wrapper><SomeComponent /></Wrapper>);
expect(screen.getByText('SomeComponent')).toBeInTheDocument();
`,
Expand Down Expand Up @@ -395,5 +401,24 @@ ruleTester.run(RULE_NAME, rule, {
},
],
},
...EVENT_HANDLER_METHODS.map<RuleInvalidTestCase>((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',
},
],
})),
]),
});