Skip to content

no-node-access rule output false-positive warning after 7.5.0 release #1024

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

Closed
Simek opened this issue Jun 9, 2025 · 6 comments
Closed

no-node-access rule output false-positive warning after 7.5.0 release #1024

Simek opened this issue Jun 9, 2025 · 6 comments
Labels
bug Something isn't working released

Comments

@Simek
Copy link

Simek commented Jun 9, 2025

Have you read the Troubleshooting section?

Yes

Plugin version

7.5.0

ESLint version

9.28.0

Node.js version

22.15.0

Bug description

When upgrading the eslint-plugin-testing-library to the latest release today I have found out few new ESLint no-node-access rule warnings which seems to be a false-positives.

Steps to reproduce

import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

it('outputs false-positive warnings', async () => {
  render(<button>Test</button>);

  const user = userEvent.setup();
  await user.click(screen.getByText('Test'));
});

Error output/screenshots

Image

ESLint configuration

ESLint flat config chunk related to Testing Library:

  {
    files: ['**/*-test.[jt]s?(x)'],
    ...testingLibrary.configs['flat/react'],
  }

Rule(s) affected

no-node-access

Anything else?

No response

Do you want to submit a pull request to fix this bug?

No

@Simek Simek added bug Something isn't working triage Pending to be triaged by a maintainer labels Jun 9, 2025
@Belco90 Belco90 removed the triage Pending to be triaged by a maintainer label Jun 9, 2025
@Belco90
Copy link
Member

Belco90 commented Jun 9, 2025

@Simek thanks for reporting.

@y-hsgw do you think you can handle this one? I think just adding "user" to EVENTS_SIMULATORS should be enough.

@Simek
Copy link
Author

Simek commented Jun 9, 2025

What if I name the constant differently? I think it's a bit too much to expect that everyone will call it the same.

Can we instead validate the typing on which the event trigger is called? In this case it would be UserEvent coming from @testing-library/user-event package.

@G-Rath
Copy link
Contributor

G-Rath commented Jun 9, 2025

Can we instead validate the typing on which the event trigger is called?

That would make TypeScript a requirement for this to be usable, which not everyone uses (though an enhanced version of this rule for TypeScript-based projects would be a cool addition)

You could improve accuracy by using logic similar to what we do in eslint-plugin-jest with our parseJestFnCall utility, which reasonably accurately resolves Jest functions in vanilla JS contexts - it works by taking a call expression and attempts to resolve it to where it comes "into" the file.

i.e.

import { test as somethingCompletelyDifferent } from '@jest/globals`;

const oneAlias = somethingCompletelyDifferent;
const twoAlias = oneAlias;
const threeAlias = twoAlias;

describe('a weird number of aliases', () => {
  threeAlias('this is a test', () => {
    // ...
  });
});

Our utility will correctly determine that both describe and threeAlias are the Jest describe and test methods respectively, which is very similar to what you want to do for this except you don't need to handle globals and you'd want to look for xyz.setup() in the middle (so you'd first trace a variable back to the setup call, then trace the subject of the call to see if it was imported from @testing-library/user-event)

There are other improvements that could be made afterwards too, for example while it'd be rare, you could easily also check if the subject has an explicit UserEvent type on its declaration (since that's part of the AST if the consumer is using @typescript-eslint/parser, so does not require the TypeScript service to leverage)

@G-Rath
Copy link
Contributor

G-Rath commented Jun 9, 2025

I would say though in the meantime it would be good to get a patch doing what @Belco90 suggested since right now this is flagging very valid code (I've got 194 errors in one project alone, just by updating - all of which are false positives 😅)

Copy link

🎉 This issue has been resolved in version 7.5.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@Belco90
Copy link
Member

Belco90 commented Jun 10, 2025

What if I name the constant differently? I think it's a bit too much to expect that everyone will call it the same.

That's the ideal fix indeed, but wanted to exclude user in the meantime to handle the expected name at least.

@G-Rath thanks for sharing how is handled in eslint-plugin-jest! There are other parts of this plugin where we should be doing the same, so it's not just a matter of updating the no-node-access rule but detecting renames/remappings in Testing Library in general.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
Development

No branches or pull requests

3 participants