☂️ eslint-plugin-react-hooks

[Boshen]

Warning

This comment is maintained by CI. Do not edit this comment directly.
To update comment template, see https://github.com/oxc-project/oxc/tree/main/tasks/lint_rules

This is tracking issue for eslint-plugin-react-hooks.

There are 2(+ 0 deprecated) rules.

• 2/2 recommended rules are remaining as TODO
• 0/0 not recommended rules are remaining as TODO
  • All done! 🎉

To get started, run the following command:

just new-react-hooks-rule <RULE_NAME>

Then register the rule in crates/oxc_linter/src/rules.rs and also declare_all_lint_rules at the bottom.

Recommended rules

✨: 0, 🚫: 0 / total: 2

Status	Name	Docs
	react-hooks/rules-of-hooks	https://reactjs.org/docs/hooks-rules.html
	react-hooks/exhaustive-deps	facebook/react#14920

✨ = Implemented, 🚫 = Not supported

[rzvxa]

@Boshen I wanted to implement these, I've noticed that the command just new-react-hooks-rule <RULE_NAME> doesn't work; I can create the rule manually I just need to know what should I change in order to keep this comment in sync in CI.

[rzvxa]

I just noticed that #2637 is a thing, I'm going to only work on the hook rules.

[Boshen]

I just noticed that #2637 is a thing, I'm going to only work on the hook rules.

#2637 is stale, maybe you can help out and merge the code first, then improve upon it.

[Boshen]

@Boshen I wanted to implement these, I've noticed that the command just new-react-hooks-rule <RULE_NAME> doesn't work; I can create the rule manually I just need to know what should I change in order to keep this comment in sync in CI.

There are only 2 rules, and the tests cannot be parsed automatically :-/

[rzvxa]

@Boshen I wanted to implement these, I've noticed that the command just new-react-hooks-rule <RULE_NAME> doesn't work; I can create the rule manually I just need to know what should I change in order to keep this comment in sync in CI.

There are only 2 rules, and the tests cannot be parsed automatically :-/

Should I put them under react or create a new category? It won't be such a bad thing to unify such categories and not base them strictly on the origin plugin.

[Boshen]

@Boshen I wanted to implement these, I've noticed that the command just new-react-hooks-rule <RULE_NAME> doesn't work; I can create the rule manually I just need to know what should I change in order to keep this comment in sync in CI.

There are only 2 rules, and the tests cannot be parsed automatically :-/

Should I put them under react or create a new category? It won't be such a bad thing to unify such categories and not base them strictly on the origin plugin.

Yeah, under the react plugin should be fine.

[rzvxa]

✨: 0, 🚫: 0 / total: 2
Status Name Docs
react-hooks/rules-of-hooks https://reactjs.org/docs/hooks-rules.html
react-hooks/exhaustive-deps facebook/react#14920

This didn't update, Have I missed something with the tasks?

PS: please don't just fix it, Let me know what I did wrong so I don't repeat it in the future.

[leaysgur]

That's because react-hooks/* rules are defined under react:: mod, it should be react_hooks:: to be tracked by lint_rules task.

oxc/crates/oxc_linter/src/rules.rs

Line 643 in 7ed673d

react::rules_of_hooks,

If this is intended, we need to update lint_rules task to mange these conversion.

[rzvxa]

That's because react-hooks/* rules are defined under react:: mod, it should be react_hooks:: to be tracked by lint_rules task.

oxc/crates/oxc_linter/src/rules.rs

Line 643 in 7ed673d

react::rules_of_hooks,

If this is intended, we need to update lint_rules task to mange these conversion.

Thanks, Yes it was intentional there are only 2 rules for hooks and I've spoken with @Boshen about merging them into react; And he gave the green light for it.

[Boshen]

I'm in favor of modifying the task script.

[rzvxa]

Can you help me with the change?

Should I do it in such a way that this issue gets closed in favor of adding these entries to #1022, Or keep this issue and just resolve the implementation from the react module?

[leaysgur]

FYI: If

keep this issue and just resolve the implementation from the react module?

diff --git a/tasks/lint_rules/src/oxlint-rules.cjs b/tasks/lint_rules/src/oxlint-rules.cjs
index 65302b62..e8ee28c5 100644
--- a/tasks/lint_rules/src/oxlint-rules.cjs
+++ b/tasks/lint_rules/src/oxlint-rules.cjs
@@ -90,7 +90,12 @@ exports.createRuleEntries = (loadedAllRules) => {
 exports.updateImplementedStatus = async (ruleEntries) => {
   const implementedRuleNames = await readAllImplementedRuleNames();

-  for (const name of implementedRuleNames) {
+  for (let name of implementedRuleNames) {
+    // These rules are implemented under `react::` mod, but issue is separated
+    if (name === "react/rules-of-hooks" || name === "react/exhaustive-deps") {
+      name = name.replace("react/", "react-hooks/");
+    }
+
     const rule = ruleEntries.get(name);
     if (rule) rule.isImplemented = true;
     else console.log(`👀 ${name} is implemented but not found in their rules`);

If not, there is a little more work to do. 😅

[rzvxa]

If not, there is a little more work to do. 😅

You seem to have a much clearer idea of this, Do you think it is possible to add react-hook plugins as react here so we can add it to the react issue(in case we even want to)?

oxc/tasks/lint_rules/src/oxlint-rules.cjs

Lines 64 to 87 in 34dd53c

	exports.createRuleEntries = (loadedAllRules) => {
	/** @type {RuleEntries} */
	const rulesEntry = new Map();
	for (const [name, rule] of loadedAllRules) {
	// Default eslint rules are not prefixed
	const prefixedName = name.includes("/") ? name : `eslint/${name}`;
	const docsUrl = rule.meta?.docs?.url ?? "";
	const isDeprecated = rule.meta?.deprecated ?? false;
	const isRecommended = rule.meta?.docs?.recommended ?? false;
	rulesEntry.set(prefixedName, {
	docsUrl,
	isDeprecated,
	isRecommended,
	// Will be updated later
	isImplemented: false,
	isNotSupported: false,
	});
	}
	return rulesEntry;
	};

---

Edit:

This way we can have something like this in the definition:

  ["react-hooks", { npm: "eslint-plugin-react-hooks", issueNo: 2174, alias: "react" }],

[leaysgur]

(Actually, I am the original author of this task. 🤫 )

I was thinking that there are a few possible ways to handle this.
If it's okay to address it (maybe) this weekend, I can take care of it.

[rzvxa]

(Actually, I am the original author of this task. 🤫 )

Makes sense😅

Let me know if there is anything that I can help with, especially if you didn't have the time to do it.