-
-
Notifications
You must be signed in to change notification settings - Fork 477
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
☂️ eslint-plugin-react-hooks #2174
Comments
@Boshen I wanted to implement these, I've noticed that the command |
I just noticed that #2637 is a thing, I'm going to only work on the hook rules. |
There are only 2 rules, and the tests cannot be parsed automatically :-/ |
Should I put them under |
Yeah, under the react plugin should be fine. |
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. |
That's because oxc/crates/oxc_linter/src/rules.rs Line 643 in 7ed673d
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. |
I'm in favor of modifying the task script. |
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 |
FYI: If
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. 😅 |
You seem to have a much clearer idea of this, Do you think it is possible to add oxc/tasks/lint_rules/src/oxlint-rules.cjs Lines 64 to 87 in 34dd53c
Edit:This way we can have something like this in the definition: ["react-hooks", { npm: "eslint-plugin-react-hooks", issueNo: 2174, alias: "react" }], |
(Actually, I am the original author of this task. 🤫 ) I was thinking that there are a few possible ways to handle this. |
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. |
Fixes #2174 , now `react-hooks` plugin and its rules are managed by `react` plugin issue.
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.
To get started, run the following command:
Then register the rule in
crates/oxc_linter/src/rules.rs
and alsodeclare_all_lint_rules
at the bottom.Recommended rules
✨: 0, 🚫: 0 / total: 2
✨ = Implemented, 🚫 = Not supported
The text was updated successfully, but these errors were encountered: