Skip to content

feat(no-node-access): add allowContainerFirstChild option #611

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

Merged
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
19 changes: 19 additions & 0 deletions docs/rules/no-node-access.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,25 @@ within(signinModal).getByPlaceholderText('Username');
document.getElementById('submit-btn').closest('button');
```

## Options

This rule has one option:

- `allowContainerFirstChild`: **disabled by default**. When we have container
with rendered content then the easiest way to access content itself is [by using
`firstChild` property](https://testing-library.com/docs/react-testing-library/api/#container-1). Use this option in cases when this is hardly avoidable.

```js
"testing-library/no-node-access": ["error", {"allowContainerFirstChild": true}]
```

Correct:

```jsx
const { container } = render(<MyComponent />);
expect(container.firstChild).toMatchSnapshot();
```

## Further Reading

### Properties / methods that return another Node
Expand Down
25 changes: 21 additions & 4 deletions lib/rules/no-node-access.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { ALL_RETURNING_NODES } from '../utils';

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

export default createTestingLibraryRule<Options, MessageIds>({
name: RULE_NAME,
Expand All @@ -25,11 +25,24 @@ export default createTestingLibraryRule<Options, MessageIds>({
noNodeAccess:
'Avoid direct Node access. Prefer using the methods from Testing Library.',
},
schema: [],
schema: [
{
type: 'object',
properties: {
allowContainerFirstChild: {
type: 'boolean',
},
},
},
],
},
defaultOptions: [],
defaultOptions: [
{
allowContainerFirstChild: false,
},
],

create(context, _, helpers) {
create(context, [{ allowContainerFirstChild = false }], helpers) {
function showErrorForNodeAccess(node: TSESTree.MemberExpression) {
// This rule is so aggressive that can cause tons of false positives outside test files when Aggressive Reporting
// is enabled. Because of that, this rule will skip this mechanism and report only if some Testing Library package
Expand All @@ -42,6 +55,10 @@ export default createTestingLibraryRule<Options, MessageIds>({
ASTUtils.isIdentifier(node.property) &&
ALL_RETURNING_NODES.includes(node.property.name)
) {
if (allowContainerFirstChild && node.property.name === 'firstChild') {
return;
}

context.report({
node,
loc: node.property.loc.start,
Expand Down
91 changes: 62 additions & 29 deletions tests/lib/rules/no-node-access.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
import rule, { RULE_NAME } from '../../../lib/rules/no-node-access';
import type { TSESLint } from '@typescript-eslint/utils';

import rule, { RULE_NAME, Options } from '../../../lib/rules/no-node-access';
import { createRuleTester } from '../test-utils';

const ruleTester = createRuleTester();

type ValidTestCase = TSESLint.ValidTestCase<Options>;

const SUPPORTED_TESTING_FRAMEWORKS = [
'@testing-library/angular',
'@testing-library/react',
Expand All @@ -11,51 +15,52 @@ const SUPPORTED_TESTING_FRAMEWORKS = [
];

ruleTester.run(RULE_NAME, rule, {
valid: SUPPORTED_TESTING_FRAMEWORKS.flatMap((testingFramework) => [
{
code: `
valid: SUPPORTED_TESTING_FRAMEWORKS.flatMap<ValidTestCase>(
(testingFramework) => [
{
code: `
import { screen } from '${testingFramework}';

const buttonText = screen.getByText('submit');
`,
},
{
code: `
},
{
code: `
import { screen } from '${testingFramework}';

const { getByText } = screen
const firstChild = getByText('submit');
expect(firstChild).toBeInTheDocument()
`,
},
{
code: `
},
{
code: `
import { screen } from '${testingFramework}';

const firstChild = screen.getByText('submit');
expect(firstChild).toBeInTheDocument()
`,
},
{
code: `
},
{
code: `
import { screen } from '${testingFramework}';

const { getByText } = screen;
const button = getByRole('button');
expect(button).toHaveTextContent('submit');
`,
},
{
code: `
},
{
code: `
import { render, within } from '${testingFramework}';

const { getByLabelText } = render(<MyComponent />);
const signInModal = getByLabelText('Sign In');
within(signInModal).getByPlaceholderText('Username');
`,
},
{
code: `
},
{
code: `
// case: code not related to testing library at all
ReactDOM.render(
<CommProvider useDsa={false}>
Expand All @@ -70,25 +75,36 @@ ruleTester.run(RULE_NAME, rule, {
document.getElementById('root')
);
`,
},
{
settings: {
'testing-library/utils-module': 'test-utils',
},
code: `
{
settings: {
'testing-library/utils-module': 'test-utils',
},
code: `
// case: custom module set but not imported (aggressive reporting limited)
const closestButton = document.getElementById('submit-btn').closest('button');
expect(closestButton).toBeInTheDocument();
`,
},
{
code: `
},
{
code: `
// case: without importing TL (aggressive reporting skipped)
const closestButton = document.getElementById('submit-btn')
expect(closestButton).toBeInTheDocument();
`,
},
]),
},
{
options: [{ allowContainerFirstChild: true }],
code: `
import { render } from '${testingFramework}';

const { container } = render(<MyComponent />)

expect(container.firstChild).toMatchSnapshot()
`,
},
]
),
invalid: SUPPORTED_TESTING_FRAMEWORKS.flatMap((testingFramework) => [
{
settings: {
Expand Down Expand Up @@ -291,5 +307,22 @@ ruleTester.run(RULE_NAME, rule, {
},
],
},
{
code: `
import { render } from '${testingFramework}';

const { container } = render(<MyComponent />)

expect(container.firstChild).toMatchSnapshot()
`,
errors: [
{
// error points to `firstChild`
line: 6,
column: 26,
messageId: 'noNodeAccess',
},
],
},
]),
});