Skip to content

Move rules settings to ESLint shared config #198

Closed
@Belco90

Description

@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?

  1. 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).

  2. 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
  1. 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
  1. 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
  1. 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

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

Metadata

Metadata

Assignees

Labels

BREAKING CHANGEThis change will require a major version bump

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions