Skip to content

Move rules settings to ESLint shared config: part 1 - utils detection #237

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

Merged
merged 11 commits into from
Oct 20, 2020
1 change: 1 addition & 0 deletions .npmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package-lock=false
10 changes: 8 additions & 2 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,17 @@ module.exports = {
statements: 100,
},
// TODO drop this custom threshold in v4
"./lib/node-utils.ts": {
'./lib/detect-testing-library-utils.ts': {
branches: 50,
functions: 90,
lines: 90,
statements: 90,
},
'./lib/node-utils.ts': {
branches: 90,
functions: 90,
lines: 90,
statements: 90,
}
},
},
};
38 changes: 38 additions & 0 deletions lib/create-testing-library-rule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { ESLintUtils, TSESLint } from '@typescript-eslint/experimental-utils';
import { getDocsUrl } from './utils';
import {
detectTestingLibraryUtils,
DetectionHelpers,
} from './detect-testing-library-utils';

// These 2 types are copied from @typescript-eslint/experimental-utils
type CreateRuleMetaDocs = Omit<TSESLint.RuleMetaDataDocs, 'url'>;
type CreateRuleMeta<TMessageIds extends string> = {
docs: CreateRuleMetaDocs;
} & Omit<TSESLint.RuleMetaData<TMessageIds>, 'docs'>;

export function createTestingLibraryRule<
TOptions extends readonly unknown[],
TMessageIds extends string,
TRuleListener extends TSESLint.RuleListener = TSESLint.RuleListener
>(
config: Readonly<{
name: string;
meta: CreateRuleMeta<TMessageIds>;
defaultOptions: Readonly<TOptions>;
create: (
context: Readonly<TSESLint.RuleContext<TMessageIds, TOptions>>,
optionsWithDefault: Readonly<TOptions>,
detectionHelpers: Readonly<DetectionHelpers>
) => TRuleListener;
}>
): TSESLint.RuleModule<TMessageIds, TOptions, TRuleListener> {
const { create, ...remainingConfig } = config;

return ESLintUtils.RuleCreator(getDocsUrl)({
...remainingConfig,
create: detectTestingLibraryUtils<TOptions, TMessageIds, TRuleListener>(
create
),
Comment on lines +34 to +36
Copy link
Member Author

Choose a reason for hiding this comment

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

Where all the magic happens now. It looks so simple after all the time I spent on it that I feel kinda stupid.

Copy link
Member

Choose a reason for hiding this comment

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

Nice!
It actually looks easier and cleaner than expected ... that's easy to say when it's written.

Copy link
Collaborator

Choose a reason for hiding this comment

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

this looks great! Great job!

});
}
65 changes: 65 additions & 0 deletions lib/detect-testing-library-utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';

export type DetectionHelpers = {
getIsImportingTestingLibrary: () => boolean;
};

/**
* Enhances a given rule `create` with helpers to detect Testing Library utils.
*/
export function detectTestingLibraryUtils<
TOptions extends readonly unknown[],
TMessageIds extends string,
TRuleListener extends TSESLint.RuleListener = TSESLint.RuleListener
>(
ruleCreate: (
context: Readonly<TSESLint.RuleContext<TMessageIds, TOptions>>,
optionsWithDefault: Readonly<TOptions>,
detectionHelpers: Readonly<DetectionHelpers>
) => TRuleListener
) {
return (
context: Readonly<TSESLint.RuleContext<TMessageIds, TOptions>>,
optionsWithDefault: Readonly<TOptions>
): TRuleListener => {
let isImportingTestingLibrary = false;

// TODO: init here options based on shared ESLint config

// helpers for Testing Library detection
const helpers: DetectionHelpers = {
getIsImportingTestingLibrary() {
return isImportingTestingLibrary;
},
};

// instructions for Testing Library detection
const detectionInstructions: TSESLint.RuleListener = {
ImportDeclaration(node: TSESTree.ImportDeclaration) {
isImportingTestingLibrary = /testing-library/g.test(
node.source.value as string
);
},
};

// update given rule to inject Testing Library detection
const ruleInstructions = ruleCreate(context, optionsWithDefault, helpers);
const enhancedRuleInstructions = Object.assign({}, ruleInstructions);

Object.keys(detectionInstructions).forEach((instruction) => {
(enhancedRuleInstructions as TSESLint.RuleListener)[instruction] = (
node
) => {
if (instruction in detectionInstructions) {
detectionInstructions[instruction](node);
}

if (ruleInstructions[instruction]) {
return ruleInstructions[instruction](node);
}
};
});

return enhancedRuleInstructions;
};
}
4 changes: 2 additions & 2 deletions lib/node-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export function isImportSpecifier(
export function isImportNamespaceSpecifier(
node: TSESTree.Node
): node is TSESTree.ImportNamespaceSpecifier {
return node?.type === AST_NODE_TYPES.ImportNamespaceSpecifier
return node?.type === AST_NODE_TYPES.ImportNamespaceSpecifier;
}

export function isImportDefaultSpecifier(
Expand Down Expand Up @@ -145,7 +145,7 @@ export function isReturnStatement(
export function isArrayExpression(
node: TSESTree.Node
): node is TSESTree.ArrayExpression {
return node?.type === AST_NODE_TYPES.ArrayExpression
return node?.type === AST_NODE_TYPES.ArrayExpression;
}

export function isAwaited(node: TSESTree.Node): boolean {
Expand Down
20 changes: 6 additions & 14 deletions lib/rules/no-node-access.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
import { getDocsUrl, ALL_RETURNING_NODES } from '../utils';
import { TSESTree } from '@typescript-eslint/experimental-utils';
import { ALL_RETURNING_NODES } from '../utils';
import { isIdentifier } from '../node-utils';
import { createTestingLibraryRule } from '../create-testing-library-rule';

export const RULE_NAME = 'no-node-access';
export type MessageIds = 'noNodeAccess';
type Options = [];

export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
export default createTestingLibraryRule<Options, MessageIds>({
Copy link
Member Author

Choose a reason for hiding this comment

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

Just creating rules like this everything is hooked and helpers are passed!

name: RULE_NAME,
meta: {
type: 'problem',
Expand All @@ -24,19 +25,11 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
},
defaultOptions: [],

create(context) {
let isImportingTestingLibrary = false;

function checkTestingEnvironment(node: TSESTree.ImportDeclaration) {
isImportingTestingLibrary = /testing-library/g.test(
node.source.value as string
);
}

create: (context, _, helpers) => {
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 gets automatic types as I wanted originally 💯

function showErrorForNodeAccess(node: TSESTree.MemberExpression) {
isIdentifier(node.property) &&
ALL_RETURNING_NODES.includes(node.property.name) &&
isImportingTestingLibrary &&
helpers.getIsImportingTestingLibrary() &&
context.report({
node: node,
loc: node.property.loc.start,
Expand All @@ -45,7 +38,6 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
}

return {
['ImportDeclaration']: checkTestingEnvironment,
['ExpressionStatement MemberExpression']: showErrorForNodeAccess,
['VariableDeclarator MemberExpression']: showErrorForNodeAccess,
};
Expand Down
2 changes: 1 addition & 1 deletion lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,5 +128,5 @@ export {
METHODS_RETURNING_NODES,
ALL_RETURNING_NODES,
PRESENCE_MATCHERS,
ABSENCE_MATCHERS
ABSENCE_MATCHERS,
};
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
"postbuild": "cpy README.md ./dist && cpy package.json ./dist && cpy LICENSE ./dist",
"lint": "eslint . --ext .js,.ts",
"lint:fix": "npm run lint -- --fix",
"format": "prettier --write README.md {lib,docs,tests}/**/*.{js,ts,md}",
"format:check": "prettier --check README.md {lib,docs,tests}/**/*.{js,json,yml,ts,md}",
"format": "prettier --write README.md \"{lib,docs,tests}/**/*.{js,ts,md}\"",
"format:check": "prettier --check README.md \"{lib,docs,tests}/**/*.{js,json,yml,ts,md}\"",
"test:local": "jest",
"test:ci": "jest --coverage",
"test:update": "npm run test:local -- --u",
Expand Down
12 changes: 6 additions & 6 deletions tests/lib/rules/await-async-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ ruleTester.run(RULE_NAME, rule, {
});
`,
})),
...ASYNC_UTILS.map(asyncUtil => ({
...ASYNC_UTILS.map((asyncUtil) => ({
code: `
import { ${asyncUtil} } from '@testing-library/dom';
test('${asyncUtil} util used in with Promise.all() does not trigger an error', async () => {
Expand All @@ -131,7 +131,7 @@ ruleTester.run(RULE_NAME, rule, {
});
`,
})),
...ASYNC_UTILS.map(asyncUtil => ({
...ASYNC_UTILS.map((asyncUtil) => ({
code: `
import { ${asyncUtil} } from '@testing-library/dom';
test('${asyncUtil} util used in with Promise.all() with an await does not trigger an error', async () => {
Expand All @@ -142,7 +142,7 @@ ruleTester.run(RULE_NAME, rule, {
});
`,
})),
...ASYNC_UTILS.map(asyncUtil => ({
...ASYNC_UTILS.map((asyncUtil) => ({
code: `
import { ${asyncUtil} } from '@testing-library/dom';
test('${asyncUtil} util used in with Promise.all() with ".then" does not trigger an error', async () => {
Expand All @@ -162,7 +162,7 @@ ruleTester.run(RULE_NAME, rule, {
waitForElementToBeRemoved(() => document.querySelector('div.getOuttaHere')),
])
});
`
`,
},
{
code: `
Expand Down Expand Up @@ -191,8 +191,8 @@ ruleTester.run(RULE_NAME, rule, {
await foo().then(() => baz())
])
})
`
}
`,
},
],
invalid: [
...ASYNC_UTILS.map((asyncUtil) => ({
Expand Down
Loading