V4 plugin runs against code

[DavidHenri008]

I upgraded to the latest version, 4.0.0, and I cannot build my code anymore since the plugin finds errors in my code.

With the following configuration :

extends: ['airbnb', 'plugin:prettier/recommended', 'plugin:testing-library/react'],
plugins: ['react-hooks', 'testing-library'],

I am getting the no-node-access error for the code document.getElementById('root') :

ReactDOM.render(
  <CommProvider useDsa={false}>
    <ThemeProvider>
      <GlobalStyle />
      <Suspense fallback={<Loader />}>
        <AppLogin />
      </Suspense>
    </ThemeProvider>
  </CommProvider>,
  document.getElementById('root')
);

I was expecting the rules to be applied to only .test.js files.

[9still]

Running into the same. Tons of false positives with any method called .getById & .getByIds. Looks like it's caused by the new aggressive reporting, but no clear documentation on recommended way to opt out or disable the rule.

testing-library/prefer-screen-queries & testing-library/no-node-access are the rules with most false positives.

[9still]

Just to provide more detail:

testing-library/utils-module
The name of your custom utility file from where you re-export everything from Testing Library package. Relates to Aggressive Reporting - Imports.

// .eslintrc
{
  "settings": {
    "testing-library/utils-module": "my-custom-test-utility-file"
  }
}

There is no information on how to revert to previous behavior when you don't have a custom utility file but just want to use testing-library/*.

@Belco90 given how aggressive this check is, wouldn't it more reasonable to assume by default that most users won't have the specialized utils? Otherwise you're introducing a ton of unpredictable fragility, since stuff will randomly break on ppl when they happen to introduce a new unrelated call with a name that the plugin finds suspicious. Would you consider reversing the default or providing a simple way to manually opt out of this feature?

It's pretty simple to use overrides to only apply the plugin to .test.js files, but that seems like the wrong thing to do, especially since you'll have tests that want to use these function name patterns for stuff that's completely unrelated & it feels wrong for testing-library to assume that it has an exclusive on using these names...

[Belco90]

Hey there! Sorry for the issues the Aggressive Reporting is causing. We should have clarified how to run eslint-plugin-testing-library only for testing files.

With the default config mentioned on the README the plugin will be run against all your files indeed. We thought about trying to run the rules only against test files, but that's out of the scope of the plugin since 1) each user can potentially have a different setup for their testing files and 2) ESLint already handles this situation.

Instead, you should use ESLint overrides to run the rules from this plugin against your test files. Assuming you are using the same pattern for your test files as Jest by default, this config would run eslint-plugin-testing-library only against test files:

// .eslintrc
{
  // testing library loaded but not enabled here since this it would be globally applied
  extends: ['airbnb', 'plugin:prettier/recommended'],
  plugins: ['react-hooks', 'testing-library'], 

  overrides: [
    {
      files: ['**/__tests__/**/*.[jt]s?(x)', '**/?(*.)+(spec|test).[jt]s?(x)'],
      // enabled only for matching files
      extends: ['plugin:testing-library/react'],
    }
  ]
}

I'll create a PR to include this info in the main installation info in the README. Additionally, no-node-access shouldn't report anything accessed from document, so I'll take that as a bug.

[9still]

@Belco90 thanks for the quick response, but it feels wrong to use test file names to apply these rules. Application code is very likely to use .getById/.getByids patterns for libraries unrelated to testing-library. While the config you list will indeed take care of the false positives in application code itself, it will still create problems for anyone trying to interact with these calls in their test files, unless they explicitly specify testing-library/utils-module.

I left a comment on #222 describing why I think the new default behavior is the wrong choice, but I don't expect you to change the default because of one unhappy user :)

I am hoping you will consider adding a way to reverse the behavior via configuration to entirely opt out of aggressive checks & opt in for specific utils.

Thank you!

[Belco90]

@9still no problem at all, I understand your frustration and I'm sorry about this. However I have to admit that we opted for this in favor of avoiding false negatives: we preferred to get developers opening issues to figure out how to opt out some Aggressive Reporting since they can spot false positives, rather than having developers without realizing they need to set up something else to get the plugin working correctly.

Having said that, that doesn't mean we have put the necessary documentation and config options in place to do so. So apart from clarifying how to make the plugin to be run only on desired test files, we can add extra config to disable the Aggressive Reporting entirely.

Aggressive Reporting has 3 mechanisms, and 2 of them can be disabled already: the module from where utils are imported, and the render methods. It's not really clear how to disable them though, so I think it's a good idea to include a new "off" option for both which opt-outs those 2 mechanisms.

Additionally, for the 3rd mechanism of Aggressive Reporting (the queries), we need to add a new Shared Setting which allows us to 1) customize the list of custom queries to be reported or 2) to switch off this mechanism.

I think including these improvements we would cover all different cases.