Move rules settings to ESLint shared config

[Belco90]

As discussed in different issues and PRs, we need to find a better approach for giving the user the possibility to define which custom render methods they use for Testing Library, or which custom modules they are importing render method.

This change will imply some breaking changes, so v4 would be the perfect opportunity to address this.

The idea is to move specific rules options duplicated across the plugin to ESLint shared settings.

You can add settings object to ESLint configuration file and it will be supplied to every rule that will be executed. This may be useful if you are adding custom rules and want them to have access to the same information and be easily configurable.

For that reason, this seems the proper approach for avoid duplicating rules configs.

What things do we need to setup there?

0. As pre-requisite we need to find a way to hook enhanced rules listener with detection helpers for all rules implementations. This has two purposes: extract the common checks (e.g. is imported from Testing Library? is a valid filename?) into a shared create rule function which already prevent rules to be executed if common checks are not met (this allows rules to focus on particular checks for the rule itself without worrying about global checks), and receive a set of detection helpers to detect Testing Library utils like "isQuery" or "isRender" (this allows rules to detect Testing Library patterns easier, reducing duplicated code, complexity and errors).

1. New shared setting to setup a custom module where Testing Libraries can be imported from other than the official package itself. There is an example here in Testing Library docs. + "aggressive reporting" by default when no custom module set + add corresponding detection helper

// render method from custom module
import { render } from 'test-utils';
// ...
const view = render(<Foo />) // corresponding rules will get setting to check if this render method has to be reported

2. New shared setting to setup the name pattern of the files desired to be analyzed (by default with same config from jest) + add corresponding detection helper THIS WAS FINALLY DISCARDED

// assume the user config the plugin to just report errors from files named *.test.js

// foo.spec.js
screen.findByText('something'); // <-- we don't report anything here since the file name doesn't match with desired filename pattern

3. Moving current renderFunctions option from existing rules to shared setting, so users can config there any custom render function they want to be considered consider by eslint-plugin-testing-library to be reported + add corresponding detection helper:

// custom render method from outside
import { renderWithTheme } from 'test-utils';
// ...
const view = renderWithTheme(<Foo />) // corresponding rules will get shared setting to check if this render method has to be reported

// custom inline render method
import { render } from '@testing-library/framework';
const setUp = (props) => <Foo {...props} />
// ...
const view = setUp() // corresponding rules will get shared setting to check if this render method has to be reported

4. Apply aggressive reporting to Testing Library queries so we assume every method starting by (get|query|find)(All)?By are related to Testing Library itself, doesn't matter if they are built-in ones (e.g. getByText) or custom ones (e.g. getByIcon) + remove current rules options for customizing extra queries + add corresponding detection helper

import { render, screen } from 'test-utils';

// ...
render(<MyComponent />);
screen.findByIcon('search'); // <-- we report this for not being awaited even if it's not a built-in query

Motivation

• We can't guess other custom methods the user uses as render if called other than that so better to set it up globally (applies to 3)
• We can't guess if something belongs to Testing Library if it's imported from a module other than "testing-library" one. For this by default we consider Testing Library always imported and all matching methods as coming from it. Then, the user can setup their custom module from where they reexport Testing Library utils, so the plugin will just report errors if utils are imported from "testing-library" or custom modules. (applies to 1)
• "Aggressive reporting" is motivated by this comment so we don't opt-out the reporting if the user has custom utils not coming directly from Testing Library. Instead, if they run into issues because the plugin is reporting things out of the expected scope, they can 1) customize the methods considered as render and 2) strict the scope of analyzed files to those matching their desired pattern (applies to 2, 3)
• Related to "aggressive reporting", instead of checking just official queries combinations from Testing Library, we will check them by pattern so if they match a Testing Library pattern we report them. Again, we avoid opting-out reporting if the user has a custom query (e.g. getByIcon) but they forgot to set it up. Instead, we check if they follow TL query naming pattern and report them as any built-in query. There is nothing to configure here, for now at least. (applies to 4)

Extra things

• Update README to explain global settings
• Update README to explain "aggressive mode", its motivation and how to restrict its scope
• Update rules docs to delete references to removed options

---

Tasks

New reporting + settings requirements

• Hooking detection helpers into rule creator + fake rule to test detection mechanism: Move rules settings to ESLint shared config: part 1 - utils detection #237
• Setting for module to import TL + aggressive TL import detection + detection helper (requirement 1): Move rules settings to ESLint shared config: part 2 - check imports #239
• Setting for filename pattern + detection helper (requirement 2): Move rules settings to ESLint shared config: part 3 - check reported file name #244, refactor: use micromatch for matching files to be reported #296 removed in refactor: remove mechanism to match files to be reported #297
• Setting for custom render methods + detection helper (requirement 3): Move rules settings to ESLint shared config: Aggressive Render Reporting + refactor render-result-naming-convention #280, refactor(render-result-naming-convention): refine checks to decide if coming from Testing Library #282
• Aggressive query reporting + detection helpers (requirement 4): Move rules settings to ESLint shared config: refactor prefer-presence-queries rule #252, Move rules settings to ESLint shared config: refactor await-async-query and no-await-sync-query #260

Rules pending to be refactored

✏️ - blocked by custom render setting + detection helper (requirement 3)
❗ - blocked by aggressive query reporting + detection helper (requirement 4)
🚧 - WIP by someone

• await-async-query ❗: Move rules settings to ESLint shared config: refactor await-async-query and no-await-sync-query #260
• await-async-utils: Move rules settings to ESLint shared config: refactor await-async-utils #263
• await-fire-event: Move rules settings to ESLint shared config: refactor await-fire-event #265
• consistent-data-testid: Move rules settings to ESLint shared config: part 5 - refactor consistent-data-testid rule #248
• no-await-sync-events: Move rules settings to ESLint shared config: refactor no-await-sync-events rule #302
• no-await-sync-query ❗: Move rules settings to ESLint shared config: refactor await-async-query and no-await-sync-query #260
• no-container ✏️ : Move rules settings to ESLint shared config: refactor no-container rule  #295
• no-debug ✏️ : Move rules settings to ESLint shared config: refactor no-debug rule  #293
• no-dom-import: Move rules settings to ESLint shared config: part 4 - refactor no-dom-import rule #247
• no-manual-cleanup: Move rules settings to ESLint shared config: part 6 - refactor no-manual-cleanup rule #249
• no-multiple-assertions-wait-for: Move rules settings to ESLint shared config: refactor no-wait-for-multiple-assertions rule #301
• no-node-access: Move rules settings to ESLint shared config: part 1 - utils detection #237
• no-promise-in-fire-event: Move rules settings to ESLint shared config: refactor no-promise-in-fire-event #266
• no-render-in-setup ✏️ : Move rules settings to ESLint shared config: refactor no-render-in-setup rule #299
• no-side-effects-wait-for: Move rules settings to ESLint shared config: refactor no-wait-for-side-effects rule #300
• no-wait-for-empty-callback: Move rules settings to ESLint shared config: refactor no-wait-for-empty-callback #284
• no-wait-for-snapshot: refactor: migrate no-wait-for-snapshot to v4 #271
• prefer-explicit-assert ❗ *remove custom query option too: refactor(prefer-explicit-assert): use new utils and remove custom query option #274
• prefer-find-by: refactor: migrate prefer-find-by to v4 #270
• prefer-presence-queries ❗: Move rules settings to ESLint shared config: refactor prefer-presence-queries rule #252
• prefer-screen-queries ✏️❗: Move rules settings to ESLint shared config: refactor prefer-screen-queries rule #285
• prefer-user-event: feat: add new settings for prefer-user-event #251
• prefer-wait-for refactor: prefer-wait-for with the new settings #255
• render-result-naming-convention ✏️ : Move rules settings to ESLint shared config: Aggressive Render Reporting + refactor render-result-naming-convention #280, refactor(render-result-naming-convention): refine checks to decide if coming from Testing Library #282

[gndelia]

I think it pretty much sums up.
We should probably consider this in the tests of all rules

[Belco90]

I think it pretty much sums up.
We should probably consider this in the tests of all rules

It sounds like a good idea. Also those two utils mentioned should have a nice bunch of unit tests. Our utils don't have tests so it's a good opportunity to start with some.

[Belco90]

I'm gonna start working on it as I finished the rule I was implementing.

[gndelia]

I'm gonna start working on it as I finished the rule I was implementing.

let me know if you need help, I just created a PR and I'm open to taking more tasks; otherwise, I'll grab v3 issues 😅

[renatoagds]

I'm open to help too if you need something 😊

[Belco90]

@gndelia @renatoagds thanks guys! I think I need to work on changing the config by myself. Then, I'll update rules currently using the renderFunction option to use the new mechanism. So at that point, I think you can help me applying the same mechanism to the remaining rules. I'll let you know when I achieve stuff needed for it.