Skip to content
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

Move rules settings to ESLint shared config: refactor no-render-in-setup rule #299

Merged
merged 11 commits into from
Mar 25, 2021
32 changes: 28 additions & 4 deletions docs/rules/no-render-in-setup.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

## Rule Details

This rule disallows the usage of `render` (or a custom render function) in setup functions (`beforeEach` and `beforeAll`) in favor of moving `render` closer to test assertions.
This rule disallows the usage of `render` (or a custom render function) in testing framework setup functions (`beforeEach` and `beforeAll`) in favor of moving `render` closer to test assertions.

Examples of **incorrect** code for this rule:

Expand All @@ -20,6 +20,22 @@ it('Should have bar', () => {
});
```

```js
const setup = () => render(<MyComponent />);

beforeEach(() => {
setup();
});

it('Should have foo', () => {
expect(screen.getByText('foo')).toBeInTheDocument();
});

it('Should have bar', () => {
expect(screen.getByText('bar')).toBeInTheDocument();
});
```

```js
beforeAll(() => {
render(<MyComponent />);
Expand All @@ -44,10 +60,18 @@ it('Should have foo and bar', () => {
});
```

If you use [custom render functions](https://testing-library.com/docs/example-react-redux) then you can set a config option in your `.eslintrc` to look for these.
```js
const setup = () => render(<MyComponent />);

```
"testing-library/no-render-in-setup": ["error", {"renderFunctions": ["renderWithRedux", "renderWithRouter"]}],
beforeEach(() => {
// other stuff...
});

it('Should have foo and bar', () => {
setup();
expect(screen.getByText('foo')).toBeInTheDocument();
expect(screen.getByText('bar')).toBeInTheDocument();
});
```

If you would like to allow the use of `render` (or a custom render function) in _either_ `beforeAll` or `beforeEach`, this can be configured using the option `allowTestingFrameworkSetupHook`. This may be useful if you have configured your tests to [skip auto cleanup](https://testing-library.com/docs/react-testing-library/setup#skipping-auto-cleanup). `allowTestingFrameworkSetupHook` is an enum that accepts either `"beforeAll"` or `"beforeEach"`.
Expand Down
22 changes: 2 additions & 20 deletions lib/node-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ interface InnermostFunctionScope extends TSESLintScope.FunctionScope {
}

export function getInnermostFunctionScope(
context: RuleContext<string, []>,
context: RuleContext<string, unknown[]>,
asyncQueryNode: TSESTree.Identifier
): InnermostFunctionScope | null {
const innermostScope = ASTUtils.getInnermostScope(
Expand Down Expand Up @@ -459,24 +459,6 @@ export function getFunctionName(
);
}

// TODO: should be removed after v4 is finished
export function isRenderFunction(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing more old functions! ✨

callNode: TSESTree.CallExpression,
renderFunctions: string[]
): boolean {
// returns true for `render` and e.g. `customRenderFn`
// as well as `someLib.render` and `someUtils.customRenderFn`
return renderFunctions.some((name) => {
return (
(ASTUtils.isIdentifier(callNode.callee) &&
name === callNode.callee.name) ||
(isMemberExpression(callNode.callee) &&
ASTUtils.isIdentifier(callNode.callee.property) &&
name === callNode.callee.property.name)
);
});
}

// TODO: extract into types file?
export type ImportModuleNode =
| TSESTree.ImportDeclaration
Expand Down Expand Up @@ -568,7 +550,7 @@ export function hasClosestExpectResolvesRejects(node: TSESTree.Node): boolean {
* Gets the Function node which returns the given Identifier.
*/
export function getInnermostReturningFunction(
context: RuleContext<string, []>,
context: RuleContext<string, unknown[]>,
node: TSESTree.Identifier
):
| TSESTree.FunctionDeclaration
Expand Down
11 changes: 1 addition & 10 deletions lib/rules/no-debug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,7 @@ export default createTestingLibraryRule<Options, MessageIds>({
noDebug: 'Unexpected debug statement',
},
fixable: null,
schema: [
{
type: 'object',
properties: {
renderFunctions: {
type: 'array',
},
},
},
],
schema: [],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a leftover from refactoring no-debug rule 😓

},
defaultOptions: [],

Expand Down
139 changes: 46 additions & 93 deletions lib/rules/no-render-in-setup.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,18 @@
import { ASTUtils, TSESTree } from '@typescript-eslint/experimental-utils';
import { TESTING_FRAMEWORK_SETUP_HOOKS } from '../utils';
import {
ESLintUtils,
TSESTree,
ASTUtils,
} from '@typescript-eslint/experimental-utils';
import { getDocsUrl, TESTING_FRAMEWORK_SETUP_HOOKS } from '../utils';
import {
isLiteral,
isProperty,
isObjectPattern,
getDeepestIdentifierNode,
getFunctionName,
getInnermostReturningFunction,
isCallExpression,
isRenderFunction,
isImportSpecifier,
} from '../node-utils';
import { createTestingLibraryRule } from '../create-testing-library-rule';

export const RULE_NAME = 'no-render-in-setup';
export type MessageIds = 'noRenderInSetup';
type Options = [
{
allowTestingFrameworkSetupHook?: string;
renderFunctions?: string[];
}
];

Expand All @@ -41,127 +35,86 @@ export function findClosestBeforeHook(
return findClosestBeforeHook(node.parent, testingFrameworkSetupHooksToFilter);
}

export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
export default createTestingLibraryRule<Options, MessageIds>({
name: RULE_NAME,
meta: {
type: 'problem',
docs: {
description: 'Disallow the use of `render` in setup functions',
description:
'Disallow the use of `render` in testing frameworks setup functions',
category: 'Best Practices',
recommended: false,
},
messages: {
noRenderInSetup:
'Move `render` out of `{{name}}` and into individual tests.',
'Forbidden usage of `render` within testing framework `{{ name }}` setup',
},
fixable: null,
schema: [
{
type: 'object',
properties: {
renderFunctions: {
type: 'array',
},
allowTestingFrameworkSetupHook: {
enum: TESTING_FRAMEWORK_SETUP_HOOKS,
},
},
anyOf: [
{
required: ['renderFunctions'],
},
{
required: ['allowTestingFrameworkSetupHook'],
},
],
},
],
},
defaultOptions: [
{
renderFunctions: [],
allowTestingFrameworkSetupHook: '',
},
],

create(context, [{ renderFunctions, allowTestingFrameworkSetupHook }]) {
let renderImportedFromTestingLib = false;
let wildcardImportName: string | null = null;
create(context, [{ allowTestingFrameworkSetupHook }], helpers) {
const renderWrapperNames: string[] = [];

function detectRenderWrapper(node: TSESTree.Identifier): void {
const innerFunction = getInnermostReturningFunction(context, node);

if (innerFunction) {
renderWrapperNames.push(getFunctionName(innerFunction));
}
}

return {
// checks if import has shape:
// import * as dtl from '@testing-library/dom';
'ImportDeclaration[source.value=/testing-library/] ImportNamespaceSpecifier'(
node: TSESTree.ImportNamespaceSpecifier
) {
wildcardImportName = node.local && node.local.name;
},
// checks if `render` is imported from a '@testing-library/foo'
'ImportDeclaration[source.value=/testing-library/]'(
node: TSESTree.ImportDeclaration
) {
renderImportedFromTestingLib = node.specifiers.some((specifier) => {
return (
isImportSpecifier(specifier) && specifier.local.name === 'render'
);
});
},
[`VariableDeclarator > CallExpression > Identifier[name="require"]`](
node: TSESTree.Identifier
) {
const {
arguments: callExpressionArgs,
} = node.parent as TSESTree.CallExpression;
const testingLibImport = callExpressionArgs.find(
(args) =>
isLiteral(args) &&
typeof args.value === 'string' &&
RegExp(/testing-library/, 'g').test(args.value)
CallExpression(node) {
const testingFrameworkSetupHooksToFilter = TESTING_FRAMEWORK_SETUP_HOOKS.filter(
(hook) => hook !== allowTestingFrameworkSetupHook
);
if (!testingLibImport) {
return;
const callExpressionIdentifier = getDeepestIdentifierNode(node);
const isRenderIdentifier = helpers.isRenderUtil(
callExpressionIdentifier
);

if (isRenderIdentifier) {
detectRenderWrapper(callExpressionIdentifier);
}
const declaratorNode = node.parent
.parent as TSESTree.VariableDeclarator;

renderImportedFromTestingLib =
isObjectPattern(declaratorNode.id) &&
declaratorNode.id.properties.some(
(property) =>
isProperty(property) &&
ASTUtils.isIdentifier(property.key) &&
property.key.name === 'render'
);
},
CallExpression(node) {
let testingFrameworkSetupHooksToFilter = TESTING_FRAMEWORK_SETUP_HOOKS;
if (allowTestingFrameworkSetupHook.length !== 0) {
testingFrameworkSetupHooksToFilter = TESTING_FRAMEWORK_SETUP_HOOKS.filter(
(hook) => hook !== allowTestingFrameworkSetupHook
);
if (
!isRenderIdentifier &&
!renderWrapperNames.includes(callExpressionIdentifier.name)
) {
return;
}

const beforeHook = findClosestBeforeHook(
node,
testingFrameworkSetupHooksToFilter
);

// if `render` is imported from a @testing-library/foo or
// imported with a wildcard, add `render` to the list of
// disallowed render functions
const disallowedRenderFns =
renderImportedFromTestingLib || wildcardImportName
? ['render', ...renderFunctions]
: renderFunctions;

if (isRenderFunction(node, disallowedRenderFns) && beforeHook) {
context.report({
node,
messageId: 'noRenderInSetup',
data: {
name: beforeHook.name,
},
});
if (!beforeHook) {
return;
}

context.report({
node: callExpressionIdentifier,
messageId: 'noRenderInSetup',
data: {
name: beforeHook.name,
},
});
},
};
},
Expand Down
7 changes: 4 additions & 3 deletions lib/rules/render-result-naming-convention.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,10 @@ export default createTestingLibraryRule<Options, MessageIds>({
}

return {
'CallExpression Identifier'(node: TSESTree.Identifier) {
if (helpers.isRenderUtil(node)) {
detectRenderWrapper(node);
CallExpression(node) {
const callExpressionIdentifier = getDeepestIdentifierNode(node);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More convenient way fo getting chained identifier.

if (helpers.isRenderUtil(callExpressionIdentifier)) {
detectRenderWrapper(callExpressionIdentifier);
}
},
VariableDeclarator(node) {
Expand Down
Loading