Skip to content

Commit

Permalink
No breaking changes
Browse files Browse the repository at this point in the history
  • Loading branch information
@eps1lon
eps1lon committed Apr 6, 2024
1 parent 1d143eb commit c5b335e
Show file tree
Hide file tree
Showing 6 changed files with 295 additions and 177 deletions.
151 changes: 115 additions & 36 deletions packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@

'use strict';

const ESLintTester = require('eslint').RuleTester;
const ESLintTesterV7 = require('eslint-v7').RuleTester;
const ESLintTesterV9 = require('eslint-v9').RuleTester;
const ReactHooksESLintPlugin = require('eslint-plugin-react-hooks');
const ReactHooksESLintRule = ReactHooksESLintPlugin.rules['exhaustive-deps'];

Expand Down Expand Up @@ -8223,8 +8224,15 @@ if (!process.env.CI) {
testsTypescript.invalid = testsTypescript.invalid.filter(predicate);
}

describe('react-hooks', () => {
const languageOptions = {
describe('rules-of-hooks/exhaustive-deps', () => {
const parserOptionsV7 = {
ecmaFeatures: {
jsx: true,
},
ecmaVersion: 6,
sourceType: 'module',
};
const languageOptionsV9 = {
ecmaVersion: 6,
sourceType: 'module',
parserOptions: {
Expand All @@ -8239,13 +8247,22 @@ describe('react-hooks', () => {
invalid: [...testsFlow.invalid, ...tests.invalid],
};

new ESLintTester({
new ESLintTesterV7({
parser: require.resolve('babel-eslint'),
parserOptions: parserOptionsV7,
}).run(
'eslint: v7, parser: babel-eslint',
ReactHooksESLintRule,
testsBabelEslint
);

new ESLintTesterV9({
languageOptions: {
...languageOptions,
...languageOptionsV9,
parser: require('@babel/eslint-parser'),
},
}).run(
'parser: @babel/eslint-parser',
'eslint: v9, parser: @babel/eslint-parser',
ReactHooksESLintRule,
testsBabelEslint
);
Expand All @@ -8255,57 +8272,119 @@ describe('react-hooks', () => {
invalid: [...testsTypescript.invalid, ...tests.invalid],
};

new ESLintTester({
new ESLintTesterV7({
parser: require.resolve('@typescript-eslint/parser-v2'),
parserOptions: parserOptionsV7,
}).run(
'eslint: v9, parser: @typescript-eslint/parser@2.x',
ReactHooksESLintRule,
testsTypescriptEslintParser
);

new ESLintTesterV9({
languageOptions: {
...languageOptions,
...languageOptionsV9,
parser: require('@typescript-eslint/parser-v2'),
},
}).run(
'parser: @typescript-eslint/parser@2.x',
'eslint: v9, parser: @typescript-eslint/parser@2.x',
ReactHooksESLintRule,
testsTypescriptEslintParser
);

new ESLintTester({
new ESLintTesterV7({
parser: require.resolve('@typescript-eslint/parser-v3'),
parserOptions: parserOptionsV7,
}).run(
'eslint: v7, parser: @typescript-eslint/parser@3.x',
ReactHooksESLintRule,
testsTypescriptEslintParser
);

new ESLintTesterV9({
languageOptions: {
...languageOptions,
...languageOptionsV9,
parser: require('@typescript-eslint/parser-v3'),
},
}).run(
'parser: @typescript-eslint/parser@3.x',
'eslint: v9, parser: @typescript-eslint/parser@3.x',
ReactHooksESLintRule,
testsTypescriptEslintParser
);

new ESLintTester({
new ESLintTesterV7({
parser: require.resolve('@typescript-eslint/parser-v4'),
parserOptions: parserOptionsV7,
}).run(
'eslint: v7, parser: @typescript-eslint/parser@4.x',
ReactHooksESLintRule,
{
valid: [
...testsTypescriptEslintParserV4.valid,
...testsTypescriptEslintParser.valid,
],
invalid: [
...testsTypescriptEslintParserV4.invalid,
...testsTypescriptEslintParser.invalid,
],
}
);

new ESLintTesterV9({
languageOptions: {
...languageOptions,
...languageOptionsV9,
parser: require('@typescript-eslint/parser-v4'),
},
}).run('parser: @typescript-eslint/parser@4.x', ReactHooksESLintRule, {
valid: [
...testsTypescriptEslintParserV4.valid,
...testsTypescriptEslintParser.valid,
],
invalid: [
...testsTypescriptEslintParserV4.invalid,
...testsTypescriptEslintParser.invalid,
],
});
}).run(
'eslint: v9, parser: @typescript-eslint/parser@4.x',
ReactHooksESLintRule,
{
valid: [
...testsTypescriptEslintParserV4.valid,
...testsTypescriptEslintParser.valid,
],
invalid: [
...testsTypescriptEslintParserV4.invalid,
...testsTypescriptEslintParser.invalid,
],
}
);

new ESLintTester({
new ESLintTesterV7({
parser: require.resolve('@typescript-eslint/parser-v5'),
parserOptions: parserOptionsV7,
}).run(
'eslint: v7, parser: @typescript-eslint/parser@^5.0.0-0',
ReactHooksESLintRule,
{
valid: [
...testsTypescriptEslintParserV4.valid,
...testsTypescriptEslintParser.valid,
],
invalid: [
...testsTypescriptEslintParserV4.invalid,
...testsTypescriptEslintParser.invalid,
],
}
);

new ESLintTesterV9({
languageOptions: {
...languageOptions,
...languageOptionsV9,
parser: require('@typescript-eslint/parser-v5'),
},
}).run('parser: @typescript-eslint/parser@^5.0.0-0', ReactHooksESLintRule, {
valid: [
...testsTypescriptEslintParserV4.valid,
...testsTypescriptEslintParser.valid,
],
invalid: [
...testsTypescriptEslintParserV4.invalid,
...testsTypescriptEslintParser.invalid,
],
});
}).run(
'eslint: v9, parser: @typescript-eslint/parser@^5.0.0-0',
ReactHooksESLintRule,
{
valid: [
...testsTypescriptEslintParserV4.valid,
...testsTypescriptEslintParser.valid,
],
invalid: [
...testsTypescriptEslintParserV4.invalid,
...testsTypescriptEslintParser.invalid,
],
}
);
});
30 changes: 19 additions & 11 deletions packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,12 @@

'use strict';

const ESLintTester = require('eslint').RuleTester;
const ESLintTesterV7 = require('eslint-v7').RuleTester;
const ESLintTesterV9 = require('eslint-v9').RuleTester;
const ReactHooksESLintPlugin = require('eslint-plugin-react-hooks');
const BabelEslintParser = require('@babel/eslint-parser');
const ReactHooksESLintRule = ReactHooksESLintPlugin.rules['rules-of-hooks'];

ESLintTester.setDefaultConfig({
languageOptions: {
parser: BabelEslintParser,
ecmaVersion: 6,
sourceType: 'module',
},
});

/**
* A string template tag that removes padding from the left side of multi-line strings
* @param {Array} strings array of code strings (only one expected)
Expand Down Expand Up @@ -1464,5 +1457,20 @@ if (!process.env.CI) {
tests.invalid = tests.invalid.filter(predicate);
}

const eslintTester = new ESLintTester();
eslintTester.run('react-hooks', ReactHooksESLintRule, tests);
describe('rules-of-hooks/rules-of-hooks', () => {
new ESLintTesterV7({
parser: require.resolve('babel-eslint'),
parserOptions: {
ecmaVersion: 6,
sourceType: 'module',
},
}).run('eslint: v7', ReactHooksESLintRule, tests);

new ESLintTesterV9({
languageOptions: {
parser: BabelEslintParser,
ecmaVersion: 6,
sourceType: 'module',
},
}).run('eslint: v9', ReactHooksESLintRule, tests);
});
6 changes: 4 additions & 2 deletions packages/eslint-plugin-react-hooks/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,16 @@
},
"homepage": "https://react.dev/",
"peerDependencies": {
"eslint": "^9.0.0"
"eslint": "^3.0.0 || ^4.0.0 || ^5.0.0 || ^6.0.0 || ^7.0.0 || ^8.0.0-0 || ^9.0.0"
},
"devDependencies": {
"@babel/eslint-parser": "^7.11.4",
"@typescript-eslint/parser-v2": "npm:@typescript-eslint/parser@^2.26.0",
"@typescript-eslint/parser-v3": "npm:@typescript-eslint/parser@^3.10.0",
"@typescript-eslint/parser-v4": "npm:@typescript-eslint/parser@^4.1.0",
"@typescript-eslint/parser-v5": "npm:@typescript-eslint/parser@^5.0.0-0",
"eslint": "^9.0.0"
"babel-eslint": "^10.0.3",
"eslint-v7": "npm:eslint@^7.7.0",
"eslint-v9": "npm:eslint@^9.0.0"
}
}
53 changes: 32 additions & 21 deletions packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,27 @@ export default {
context.report(problem);
}

/**
* SourceCode#getText that also works down to ESLint 3.0.0
*/
function getSource(node) {
if (typeof context.getSource === 'function') {
return context.getSource(node);
} else {
return context.sourceCode.getText(node);
}
}
/**
* SourceCode#getScope that also works down to ESLint 3.0.0
*/
function getScope(node) {
if (typeof context.getScope === 'function') {
return context.getScope();
} else {
return context.sourceCode.getScope(node);
}
}

const scopeManager = context.getSourceCode().scopeManager;

// Should be shared between visitors.
Expand Down Expand Up @@ -526,13 +547,11 @@ export default {
node: writeExpr,
message:
`Assignments to the '${key}' variable from inside React Hook ` +
`${context.sourceCode.getText(
reactiveHook,
)} will be lost after each ` +
`${getSource(reactiveHook)} will be lost after each ` +
`render. To preserve the value over time, store it in a useRef ` +
`Hook and keep the mutable value in the '.current' property. ` +
`Otherwise, you can move this variable directly inside ` +
`${context.sourceCode.getText(reactiveHook)}.`,
`${getSource(reactiveHook)}.`,
});
}

Expand Down Expand Up @@ -632,9 +651,7 @@ export default {
reportProblem({
node: declaredDependenciesNode,
message:
`React Hook ${context.sourceCode.getText(
reactiveHook,
)} was passed a ` +
`React Hook ${getSource(reactiveHook)} was passed a ` +
'dependency list that is not an array literal. This means we ' +
"can't statically verify whether you've passed the correct " +
'dependencies.',
Expand All @@ -654,9 +671,7 @@ export default {
reportProblem({
node: declaredDependencyNode,
message:
`React Hook ${context.sourceCode.getText(
reactiveHook,
)} has a spread ` +
`React Hook ${getSource(reactiveHook)} has a spread ` +
"element in its dependency array. This means we can't " +
"statically verify whether you've passed the " +
'correct dependencies.',
Expand All @@ -668,12 +683,12 @@ export default {
node: declaredDependencyNode,
message:
'Functions returned from `useEffectEvent` must not be included in the dependency array. ' +
`Remove \`${context.sourceCode.getText(
`Remove \`${getSource(
declaredDependencyNode,
)}\` from the list.`,
suggest: [
{
desc: `Remove the dependency \`${context.sourceCode.getText(
desc: `Remove the dependency \`${getSource(
declaredDependencyNode,
)}\``,
fix(fixer) {
Expand Down Expand Up @@ -714,9 +729,7 @@ export default {
reportProblem({
node: declaredDependencyNode,
message:
`React Hook ${context.sourceCode.getText(
reactiveHook,
)} has a ` +
`React Hook ${getSource(reactiveHook)} has a ` +
`complex expression in the dependency array. ` +
'Extract it to a separate variable so it can be statically checked.',
});
Expand Down Expand Up @@ -986,7 +999,7 @@ export default {
` However, 'props' will change when *any* prop changes, so the ` +
`preferred fix is to destructure the 'props' object outside of ` +
`the ${reactiveHookName} call and refer to those specific props ` +
`inside ${context.sourceCode.getText(reactiveHook)}.`;
`inside ${getSource(reactiveHook)}.`;
}
}

Expand Down Expand Up @@ -1136,7 +1149,7 @@ export default {
reportProblem({
node: declaredDependenciesNode,
message:
`React Hook ${context.sourceCode.getText(reactiveHook)} has ` +
`React Hook ${getSource(reactiveHook)} has ` +
// To avoid a long message, show the next actionable item.
(getWarningMessage(missingDependencies, 'a', 'missing', 'include') ||
getWarningMessage(
Expand Down Expand Up @@ -1258,9 +1271,7 @@ export default {
return; // Handled
}
// We'll do our best effort to find it, complain otherwise.
const variable = context.sourceCode
.getScope(callback)
.set.get(callback.name);
const variable = getScope(callback).set.get(callback.name);
if (variable == null || variable.defs == null) {
// If it's not in scope, we don't care.
return; // Handled
Expand Down Expand Up @@ -1804,7 +1815,7 @@ function getReactiveHookCallbackIndex(calleeNode, options) {
}

/**
* ESLint won't assign node.parent to references from context.sourceCode.getScope()
* ESLint won't assign node.parent to references from context.getScope()
*
* So instead we search for the node from an ancestor assigning node.parent
* as we go. This mutates the AST.
Expand Down
Loading

0 comments on commit c5b335e

Please sign in to comment.