Improve comments and code style in x and dom rules

The changes involve adding, improving, and clarifying comments throughout various React ESLint rules, making the code more maintainable and easier to understand. This includes:

1. Adding descriptive comments to explain complex logic
2. Clarifying the purpose of various conditions and checks
3. Making code intentions more explicit
4. Improving parameter and variable descriptions
5. Documenting component lifecycle hooks and state management

The bulk of the changes are in adding comments to provide better context while leaving the actual logic unchanged. This will help developers better understand the rules' behaviors and make future maintenance easier.

Author: Rel1cx

apps/website/content/docs/rules/overview.mdx

@@ -27,7 +27,7 @@ full: true
 
 | Rule                                                                                 | ✅   |     🌟    | Description                                                                                         |  `react` |
 | :----------------------------------------------------------------------------------- | :-- | :-------: | :-------------------------------------------------------------------------------------------------- | :------: |
-| [`jsx-key-before-spread`](./jsx-key-before-spread)                                   | 1️⃣ |           | Enforces that the `key` attribute is placed before the spread attribute in JSX elements             |          |
+| [`jsx-key-before-spread`](./jsx-key-before-spread)                                   | 1️⃣ |           | Enforces that the 'key' prop is placed before the spread prop in JSX elements                       |          |
 | [`jsx-no-comment-textnodes`](./jsx-no-comment-textnodes)                             | 1️⃣ |           | Prevents comments from being inserted as text nodes                                                 |          |
 | [`jsx-no-duplicate-props`](./jsx-no-duplicate-props)                                 | 1️⃣ |           | Disallow duplicate props in JSX elements                                                            |          |
 | [`jsx-no-iife`](./jsx-no-iife)                                                       | 0️⃣ |    `🧪`   | Disallows `IIFE` in JSX elements                                                                    |          |

packages/plugins/eslint-plugin-react-dom/src/rules/no-dangerously-set-innerhtml-with-children.ts

@@ -32,33 +32,39 @@ export default createRule<[], MessageID>({
 const DSIH = "dangerouslySetInnerHTML";
 
 /**
- * Checks if a JSX child node is considered significant (i.e., not just whitespace for formatting).
- * @param node The JSX child node to check.
- * @returns `true` if the node is significant, `false` otherwise.
+ * Checks if a JSX child node is considered significant (i.e., not just whitespace for formatting)
+ * @param node The JSX child node to check
+ * @returns `true` if the node is significant, `false` otherwise
  */
 function isSignificantChildren(node: TSESTree.JSXElement["children"][number]) {
+  // Any node that is not plain text is considered significant
   if (!isJsxText(node)) {
     return true;
   }
   // A JSXText node is insignificant if it's purely whitespace and contains a newline,
   // which is a common pattern for formatting.
   const isFormattingWhitespace = node.raw.trim() === "" && node.raw.includes("\n");
 
+  // The node is significant if it's not just formatting whitespace
   return !isFormattingWhitespace;
 }
 
 export function create(context: RuleContext<MessageID, []>): RuleListener {
-  // Fast path: skip if `dangerouslySetInnerHTML` is not present in the file
+  // Fast path: if the file doesn't contain `dangerouslySetInnerHTML`, we don't need to do anything
   if (!context.sourceCode.text.includes(DSIH)) {
     return {};
   }
 
   return {
     JSXElement(node) {
       const findJsxAttribute = getJsxAttribute(context, node);
+      // Check if the element has the 'dangerouslySetInnerHTML' prop. If not, we can stop
       if (findJsxAttribute(DSIH) == null) return;
+      // Check for a 'children' prop or actual child nodes that are not just whitespace
       const childrenPropOrNode = findJsxAttribute("children") ?? node.children.find(isSignificantChildren);
+      // If no children are found, the rule passes
       if (childrenPropOrNode == null) return;
+      // If both 'dangerouslySetInnerHTML' and children are present, report an error
       context.report({
         messageId: "noDangerouslySetInnerhtmlWithChildren",
         node: childrenPropOrNode,

packages/plugins/eslint-plugin-react-dom/src/rules/no-dangerously-set-innerhtml.ts

@@ -34,9 +34,13 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
   // Fast path: skip if `dangerouslySetInnerHTML` is not present in the file
   if (!context.sourceCode.text.includes(DSIH)) return {};
   return {
+    // This function runs for each JSX element
     JSXElement(node) {
+      // Check if the element has the 'dangerouslySetInnerHTML' prop
       const dsihProp = getJsxAttribute(context, node)(DSIH);
+      // If the prop is not found, do nothing
       if (dsihProp == null) return;
+      // If the prop is found, report an error
       context.report({
         messageId: "noDangerouslySetInnerhtml",
         node: dsihProp,

packages/plugins/eslint-plugin-react-dom/src/rules/no-find-dom-node.ts

@@ -37,11 +37,13 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
     CallExpression(node) {
       const { callee } = node;
       switch (callee.type) {
+        // Handles cases like `findDOMNode()`.
         case T.Identifier:
           if (callee.name === findDOMNode) {
             context.report({ messageId: "noFindDomNode", node });
           }
           return;
+        // Handles cases like `ReactDOM.findDOMNode()`.
         case T.MemberExpression:
           if (callee.property.type === T.Identifier && callee.property.name === findDOMNode) {
             context.report({ messageId: "noFindDomNode", node });

packages/plugins/eslint-plugin-react-dom/src/rules/no-flush-sync.ts

@@ -37,11 +37,13 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
     CallExpression(node) {
       const { callee } = node;
       switch (callee.type) {
+        // Handle direct calls like `flushSync()`
         case T.Identifier:
           if (callee.name === flushSync) {
             context.report({ messageId: "noFlushSync", node });
           }
           return;
+        // Handle member access calls like `ReactDOM.flushSync()`
         case T.MemberExpression:
           if (callee.property.type === T.Identifier && callee.property.name === flushSync) {
             context.report({ messageId: "noFlushSync", node });

packages/plugins/eslint-plugin-react-dom/src/rules/no-hydrate.ts

@@ -40,14 +40,17 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
   // Fast path: skip if `hydrate` is not present in the file
   if (!context.sourceCode.text.includes(hydrate)) return {};
   const settings = getSettingsFromContext(context);
+  // This rule only applies to React 18.0.0 and later.
   if (compare(settings.version, "18.0.0", "<")) return {};
 
-  const reactDomNames = new Set<string>();
-  const hydrateNames = new Set<string>();
+  // Keep track of imports from 'react-dom'.
+  const reactDomNames = new Set<string>(); // For `import ReactDOM from 'react-dom'`
+  const hydrateNames = new Set<string>(); // For `import { hydrate } from 'react-dom'`
 
   return {
     CallExpression(node) {
       switch (true) {
+        // Case 1: Direct call to `hydrate()`.
         case node.callee.type === T.Identifier
           && hydrateNames.has(node.callee.name):
           context.report({
@@ -56,6 +59,7 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
             fix: getFix(context, node),
           });
           return;
+        // Case 2: Call on a `react-dom` import, like `ReactDOM.hydrate()`
         case node.callee.type === T.MemberExpression
           && node.callee.object.type === T.Identifier
           && node.callee.property.type === T.Identifier
@@ -71,15 +75,18 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
     },
     ImportDeclaration(node) {
       const [baseSource] = node.source.value.split("/");
+      // We only care about imports from 'react-dom'
       if (baseSource !== "react-dom") return;
       for (const specifier of node.specifiers) {
         switch (specifier.type) {
+          // `import { hydrate } from 'react-dom'`
           case T.ImportSpecifier:
             if (specifier.imported.type !== T.Identifier) continue;
             if (specifier.imported.name === hydrate) {
               hydrateNames.add(specifier.local.name);
             }
             continue;
+          // `import ReactDOM from 'react-dom'` or `import * as ReactDOM from 'react-dom'`
           case T.ImportDefaultSpecifier:
           case T.ImportNamespaceSpecifier:
             reactDomNames.add(specifier.local.name);
@@ -95,8 +102,12 @@ function getFix(context: RuleContext, node: TSESTree.CallExpression) {
   return (fixer: RuleFixer) => {
     const [arg0, arg1] = node.arguments;
     if (arg0 == null || arg1 == null) return null;
+    // The fix consists of two parts:
     return [
+      // 1. Add the new import for `hydrateRoot`
       fixer.insertTextBefore(context.sourceCode.ast, 'import { hydrateRoot } from "react-dom/client";\n'),
+      // 2. Replace `hydrate(element, container)` with `hydrateRoot(container, element)`
+      // Note that the arguments are swapped
       fixer.replaceText(node, `hydrateRoot(${getText(arg1)}, ${getText(arg0)})`),
     ];
   };

packages/plugins/eslint-plugin-react-dom/src/rules/no-missing-button-type.ts

@@ -41,20 +41,26 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
 
   return {
     JSXElement(node) {
+      // Resolve the JSX element to its corresponding DOM element type
+      // If it's not a '<button>', skip it
       if (resolver.resolve(node).domElementType !== "button") {
         return;
       }
 
+      // Check if the 'type' attribute already exists on the button element
       if (getJsxAttribute(context, node)("type") != null) {
         return;
       }
 
+      // If the 'type' attribute is missing, report an error
       context.report({
         messageId: "noMissingButtonType",
         node: node.openingElement,
+        // Provide suggestions to automatically fix the issue
         suggest: BUTTON_TYPES.map((type): RuleSuggest<MessageID> => ({
           messageId: "addButtonType",
           data: { type },
+          // The fix function inserts the 'type' attribute with a suggested value
           fix: (fixer) => fixer.insertTextAfter(node.openingElement.name, ` type="${type}"`),
         })),
       });

packages/plugins/eslint-plugin-react-dom/src/rules/no-missing-iframe-sandbox.ts

@@ -41,13 +41,13 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
   return {
     JSXElement(node) {
       const { domElementType } = resolver.resolve(node);
-      // If the element is not an iframe, we don't need to do anything.
+      // If the element is not an iframe, we don't need to do anything
       if (domElementType !== "iframe") return;
 
       // Find the 'sandbox' prop on the iframe element.
       const sandboxProp = getJsxAttribute(context, node)("sandbox");
 
-      // If the 'sandbox' prop is missing, report an error.
+      // If the 'sandbox' prop is missing, report an error
       if (sandboxProp == null) {
         context.report({
           messageId: "noMissingIframeSandbox",
@@ -56,20 +56,20 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
             messageId: "addIframeSandbox",
             data: { value: "" },
             fix(fixer) {
-              // Suggest adding a 'sandbox' attribute.
+              // Suggest adding a 'sandbox' attribute
               return fixer.insertTextAfter(node.openingElement.name, ` sandbox=""`);
             },
           }],
         });
         return;
       }
 
-      // Resolve the value of the 'sandbox' attribute.
+      // Resolve the value of the 'sandbox' attribute
       const sandboxValue = resolveJsxAttributeValue(context, sandboxProp);
-      // If the value is a static string, the prop is correctly used.
+      // If the value is a static string, the prop is correctly used
       if (typeof sandboxValue.toStatic("sandbox") === "string") return;
 
-      // If the value is not a static string (e.g., a variable), report an error.
+      // If the value is not a static string (e.g., a variable), report an error
       context.report({
         messageId: "noMissingIframeSandbox",
         node: sandboxValue.node ?? sandboxProp,
@@ -78,9 +78,9 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
             messageId: "addIframeSandbox",
             data: { value: "" },
             fix(fixer) {
-              // Do not try to fix spread attributes.
+              // Do not try to fix spread attributes
               if (sandboxValue.kind.startsWith("spread")) return null;
-              // Suggest replacing the prop with a valid one.
+              // Suggest replacing the prop with a valid one
               return fixer.replaceText(sandboxProp, `sandbox=""`);
             },
           },

packages/plugins/eslint-plugin-react-dom/src/rules/no-render-return-value.ts

@@ -11,6 +11,7 @@ export const RULE_FEATURES = [] as const satisfies RuleFeature[];
 
 export type MessageID = CamelCase<typeof RULE_NAME>;
 
+// Parent AST node types that indicate the return value of `ReactDOM.render` is being used
 const banParentTypes = [
   T.VariableDeclarator,
   T.Property,
@@ -37,25 +38,31 @@ export default createRule<[], MessageID>({
 });
 
 export function create(context: RuleContext<MessageID, []>): RuleListener {
+  // Sets to track imported names for 'ReactDOM' and 'render'
   const reactDomNames = new Set<string>(["ReactDOM", "ReactDom"]);
   const renderNames = new Set<string>();
 
   return {
+    // Checks for calls to 'render' or 'ReactDOM.render' and reports if their return value is used
     CallExpression(node) {
       switch (true) {
+        // Handles direct calls to 'render' (e.g., from `import { render } from 'react-dom'`)
         case node.callee.type === T.Identifier
           && renderNames.has(node.callee.name)
+          // Check if the return value is being used
           && banParentTypes.includes(node.parent.type):
           context.report({
             messageId: "noRenderReturnValue",
             node,
           });
           return;
+        // Handles member expression calls like 'ReactDOM.render'
         case node.callee.type === T.MemberExpression
           && node.callee.object.type === T.Identifier
           && node.callee.property.type === T.Identifier
           && node.callee.property.name === "render"
           && reactDomNames.has(node.callee.object.name)
+          // Check if the return value is being used
           && banParentTypes.includes(node.parent.type):
           context.report({
             messageId: "noRenderReturnValue",
@@ -64,17 +71,21 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
           return;
       }
     },
+    // Tracks imports from 'react-dom' to identify 'ReactDOM' and 'render' related identifiers
     ImportDeclaration(node) {
+      // Check if the import source is 'react-dom' or 'react-dom/client'
       const [baseSource] = node.source.value.split("/");
       if (baseSource !== "react-dom") return;
       for (const specifier of node.specifiers) {
         switch (specifier.type) {
+          // Handles named imports like `import { render } from 'react-dom'`
           case T.ImportSpecifier:
             if (specifier.imported.type !== T.Identifier) continue;
             if (specifier.imported.name === "render") {
               renderNames.add(specifier.local.name);
             }
             continue;
+          // Handles default or namespace imports like `import ReactDOM from 'react-dom'`
           case T.ImportDefaultSpecifier:
           case T.ImportNamespaceSpecifier:
             reactDomNames.add(specifier.local.name);

packages/plugins/eslint-plugin-react-dom/src/rules/no-render.ts

@@ -38,14 +38,19 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
   // Fast path: skip if `render` is not present in the file
   if (!context.sourceCode.text.includes("render")) return {};
   const settings = getSettingsFromContext(context);
+  // This rule only applies to React 18.0.0 and later
   if (compare(settings.version, "18.0.0", "<")) return {};
 
+  // Tracks imported names for ReactDOM, e.g., `ReactDOM` or `ReactDom`.
   const reactDomNames = new Set<string>(["ReactDOM", "ReactDom"]);
+  // Tracks imported names for the `render` function
   const renderNames = new Set<string>();
 
   return {
+    // Visitor for call expressions, e.g., render() or ReactDOM.render()
     CallExpression(node) {
       switch (true) {
+        // Case 1: Direct call to 'render', e.g., from `import { render } from 'react-dom'`
         case node.callee.type === T.Identifier
           && renderNames.has(node.callee.name):
           context.report({
@@ -54,6 +59,7 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
             fix: getFix(context, node),
           });
           return;
+        // Case 2: Member expression call, e.g., `ReactDOM.render()`
         case node.callee.type === T.MemberExpression
           && node.callee.object.type === T.Identifier
           && node.callee.property.type === T.Identifier
@@ -69,15 +75,18 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
     },
     ImportDeclaration(node) {
       const [baseSource] = node.source.value.split("/");
+      // Only consider imports from 'react-dom'
       if (baseSource !== "react-dom") return;
       for (const specifier of node.specifiers) {
         switch (specifier.type) {
+          // Handles: import { render } from 'react-dom'
           case T.ImportSpecifier:
             if (specifier.imported.type !== T.Identifier) continue;
             if (specifier.imported.name === "render") {
               renderNames.add(specifier.local.name);
             }
             continue;
+          // Handles: import ReactDOM from 'react-dom' or import * as ReactDOM from 'react-dom'
           case T.ImportDefaultSpecifier:
           case T.ImportNamespaceSpecifier:
             reactDomNames.add(specifier.local.name);
@@ -88,13 +97,22 @@ export function create(context: RuleContext<MessageID, []>): RuleListener {
   };
 }
 
+/**
+ * Provides a fixer function to replace `render(app, container)` with `createRoot(container).render(app)`
+ * @param context The rule context
+ * @param node The `CallExpression` node to fix
+ * @returns A fixer function or null if the fix cannot be applied
+ */
 function getFix(context: RuleContext, node: TSESTree.CallExpression) {
   const getText = (n: TSESTree.Node) => context.sourceCode.getText(n);
   return (fixer: RuleFixer) => {
+    // `render` takes two arguments: component and container
     const [arg0, arg1] = node.arguments;
     if (arg0 == null || arg1 == null) return null;
     return [
+      // Add `import { createRoot } from "react-dom/client";` at the top of the file
       fixer.insertTextBefore(context.sourceCode.ast, 'import { createRoot } from "react-dom/client";\n'),
+      // Replace `render(arg0, arg1)` with `createRoot(arg1).render(arg0)`
       fixer.replaceText(node, `createRoot(${getText(arg1)}).render(${getText(arg0)})`),
     ];
   };