[New] jsx-curly-brace-presence: add "propElementValues" config option

Fixes #3184
jsx-eslint · Jan 27, 2022 · ed63c01 · ed63c01
1 parent 541ea43
commit ed63c01
Show file tree

Hide file tree

Showing 4 changed files with 103 additions and 28 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
 ### Added
 * add [`hook-use-state`] rule to enforce symmetric useState hook variable names ([#2921][] @duncanbeevers)
 * [`jsx-no-target-blank`]: Improve fixer with option `allowReferrer` ([#3167][] @apepper)
+* [`jsx-curly-brace-presence`]: add "propElementValues" config option ([#3191][] @ljharb)
 
 ### Fixed
 * [`prop-types`], `propTypes`: add support for exported type inference ([#3163][] @vedadeepta)
@@ -22,6 +23,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
 * [Docs] [`display-name`]: improve examples ([#3189][] @golopot)
 * [Refactor] [`no-invalid-html-attribute`]: sort HTML_ELEMENTS and messages ([#3182][] @Primajin)
 
+[#3191]: https://github.com/yannickcr/eslint-plugin-react/pull/3191
 [#3190]: https://github.com/yannickcr/eslint-plugin-react/pull/3190
 [#3189]: https://github.com/yannickcr/eslint-plugin-react/pull/3189
 [#3186]: https://github.com/yannickcr/eslint-plugin-react/pull/3186

diff --git a/docs/rules/jsx-curly-brace-presence.md b/docs/rules/jsx-curly-brace-presence.md
@@ -8,15 +8,17 @@ For situations where JSX expressions are unnecessary, please refer to [the React
 
 ## Rule Details
 
-By default, this rule will check for and warn about unnecessary curly braces in both JSX props and children.
+By default, this rule will check for and warn about unnecessary curly braces in both JSX props and children. For the sake of backwards compatibility, prop values that are JSX elements are not considered by default.
 
-You can pass in options to enforce the presence of curly braces on JSX props or children or both. The same options are available for not allowing unnecessary curly braces as well as ignoring the check.
+You can pass in options to enforce the presence of curly braces on JSX props, children, JSX prop values that are JSX elements, or any combination of the three. The same options are available for not allowing unnecessary curly braces as well as ignoring the check.
+
+**Note**: it is _highly recommended_ that you configure this rule with an object, and that you set "propElementValues" to "always". The ability to omit curly braces around prop values that are JSX elements is obscure, and intentionally undocumented, and should not be relied upon.
 
 ## Rule Options
 
 ```js
 ...
-"react/jsx-curly-brace-presence": [<enabled>, { "props": <string>, "children": <string> }]
+"react/jsx-curly-brace-presence": [<enabled>, { "props": <string>, "children": <string>, "propElementValues": <string> }]
 ...
 ```
 
@@ -32,9 +34,9 @@ or alternatively
 
 They are `always`, `never` and `ignore` for checking on JSX props and children.
 
-* `always`: always enforce curly braces inside JSX props or/and children
-* `never`: never allow unnecessary curly braces inside JSX props or/and children
-* `ignore`: ignore the rule for JSX props or/and children
+* `always`: always enforce curly braces inside JSX props, children, and/or JSX prop values that are JSX Elements
+* `never`: never allow unnecessary curly braces inside JSX props, children, and/or JSX prop values that are JSX Elements
+* `ignore`: ignore the rule for JSX props, children, and/or JSX prop values that are JSX Elements
 
 If passed in the option to fix, this is how a style violation will get fixed
 
@@ -73,9 +75,31 @@ They can be fixed to:
 <App prop="Hello world" attr="foo" />;
 ```
 
+Examples of **incorrect** code for this rule, when configured with `{ props: "always", children: "always", "propElementValues": "always" }`:
+```jsx
+<App prop=<div /> />;
+```
+
+They can be fixed to:
+
+```jsx
+<App prop={<div />} />;
+```
+
+Examples of **incorrect** code for this rule, when configured with `{ props: "never", children: "never", "propElementValues": "never" }`:
+```jsx
+<App prop={<div />} />;
+```
+
+They can be fixed to:
+
+```jsx
+<App prop=<div /> />;
+```
+
 ### Alternative syntax
 
-The options are also `always`, `never` and `ignore` for the same meanings.
+The options are also `always`, `never`, and `ignore` for the same meanings.
 
 In this syntax, only a string is provided and the default will be set to that option for checking on both JSX props and children.
 

diff --git a/lib/rules/jsx-curly-brace-presence.js b/lib/rules/jsx-curly-brace-presence.js
@@ -25,7 +25,7 @@ const OPTION_VALUES = [
   OPTION_NEVER,
   OPTION_IGNORE,
 ];
-const DEFAULT_CONFIG = { props: OPTION_NEVER, children: OPTION_NEVER };
+const DEFAULT_CONFIG = { props: OPTION_NEVER, children: OPTION_NEVER, propElementValues: OPTION_IGNORE };
 
 // ------------------------------------------------------------------------------
 // Rule Definition
@@ -58,6 +58,7 @@ module.exports = {
             properties: {
               props: { enum: OPTION_VALUES },
               children: { enum: OPTION_VALUES },
+              propElementValues: { enum: OPTION_VALUES },
             },
             additionalProperties: false,
           },
@@ -73,7 +74,7 @@ module.exports = {
     const HTML_ENTITY_REGEX = () => /&[A-Za-z\d#]+;/g;
     const ruleOptions = context.options[0];
     const userConfig = typeof ruleOptions === 'string'
-      ? { props: ruleOptions, children: ruleOptions }
+      ? { props: ruleOptions, children: ruleOptions, propElementValues: ruleOptions }
       : Object.assign({}, DEFAULT_CONFIG, ruleOptions);
 
     function containsLineTerminators(rawStringValue) {
@@ -173,22 +174,28 @@ module.exports = {
         node: JSXExpressionNode,
         fix(fixer) {
           const expression = JSXExpressionNode.expression;
-          const expressionType = expression.type;
-          const parentType = JSXExpressionNode.parent.type;
 
           let textToReplace;
-          if (parentType === 'JSXAttribute') {
-            textToReplace = `"${expressionType === 'TemplateLiteral'
-              ? expression.quasis[0].value.raw
-              : expression.raw.substring(1, expression.raw.length - 1)
-            }"`;
-          } else if (jsxUtil.isJSX(expression)) {
+          if (jsxUtil.isJSX(expression)) {
             const sourceCode = context.getSourceCode();
-
             textToReplace = sourceCode.getText(expression);
           } else {
-            textToReplace = expressionType === 'TemplateLiteral'
-              ? expression.quasis[0].value.cooked : expression.value;
+            const expressionType = expression && expression.type;
+            const parentType = JSXExpressionNode.parent.type;
+
+            if (parentType === 'JSXAttribute') {
+              textToReplace = `"${expressionType === 'TemplateLiteral'
+                ? expression.quasis[0].value.raw
+                : expression.raw.substring(1, expression.raw.length - 1)
+              }"`;
+            } else if (jsxUtil.isJSX(expression)) {
+              const sourceCode = context.getSourceCode();
+
+              textToReplace = sourceCode.getText(expression);
+            } else {
+              textToReplace = expressionType === 'TemplateLiteral'
+                ? expression.quasis[0].value.cooked : expression.value;
+            }
           }
 
           return fixer.replaceText(JSXExpressionNode, textToReplace);
@@ -200,6 +207,10 @@ module.exports = {
       report(context, messages.missingCurly, 'missingCurly', {
         node: literalNode,
         fix(fixer) {
+          if (jsxUtil.isJSX(literalNode)) {
+            return fixer.replaceText(literalNode, `{${context.getSourceCode().getText(literalNode)}}`);
+          }
+
           // If a HTML entity name is found, bail out because it can be fixed
           // by either using the real character or the unicode equivalent.
           // If it contains any line terminator character, bail out as well.
@@ -323,7 +334,8 @@ module.exports = {
 
       return adjSiblings.some((x) => x.type && arrayIncludes(['JSXExpressionContainer', 'JSXElement'], x.type));
     }
-    function shouldCheckForUnnecessaryCurly(parent, node, config) {
+    function shouldCheckForUnnecessaryCurly(node, config) {
+      const parent = node.parent;
       // Bail out if the parent is a JSXAttribute & its contents aren't
       // StringLiteral or TemplateLiteral since e.g
       // <App prop1={<CustomEl />} prop2={<CustomEl>...</CustomEl>} />
@@ -358,6 +370,9 @@ module.exports = {
     }
 
     function shouldCheckForMissingCurly(node, config) {
+      if (jsxUtil.isJSX(node)) {
+        return config.propElementValues !== OPTION_IGNORE;
+      }
       if (
         isLineBreak(node.raw)
         || containsOnlyHtmlEntities(node.raw)
@@ -381,13 +396,19 @@ module.exports = {
     // --------------------------------------------------------------------------
 
     return {
-      JSXExpressionContainer: (node) => {
-        if (shouldCheckForUnnecessaryCurly(node.parent, node, userConfig)) {
+      'JSXAttribute > JSXExpressionContainer > JSXElement'(node) {
+        if (userConfig.propElementValues === OPTION_NEVER) {
+          reportUnnecessaryCurly(node.parent);
+        }
+      },
+
+      JSXExpressionContainer(node) {
+        if (shouldCheckForUnnecessaryCurly(node, userConfig)) {
           lintUnnecessaryCurly(node);
         }
       },
 
-      'Literal, JSXText': (node) => {
+      'JSXAttribute > JSXElement, Literal, JSXText'(node) {
         if (shouldCheckForMissingCurly(node, userConfig)) {
           reportMissingCurly(node);
         }

diff --git a/tests/lib/rules/jsx-curly-brace-presence.js b/tests/lib/rules/jsx-curly-brace-presence.js
@@ -438,7 +438,23 @@ ruleTester.run('jsx-curly-brace-presence', rule, {
           </App>
         `,
       },
-    ] : [])
+    ] : []),
+    {
+      code: `<App horror=<div /> />`,
+      features: ['no-ts'],
+    },
+    {
+      code: `<App horror={<div />} />`,
+    },
+    {
+      code: `<App horror=<div /> />`,
+      options: [{ propElementValues: 'ignore' }],
+      features: ['no-ts'],
+    },
+    {
+      code: `<App horror={<div />} />`,
+      options: [{ propElementValues: 'ignore' }],
+    }
   )),
 
   invalid: parsers.all([].concat(
@@ -851,9 +867,7 @@ ruleTester.run('jsx-curly-brace-presence', rule, {
           &nbsp;
         </App>
       `,
-      errors: [
-        { messageId: 'missingCurly' },
-      ],
+      errors: [{ messageId: 'missingCurly' }],
       options: [{ children: 'always' }],
     },
     {
@@ -889,6 +903,20 @@ ruleTester.run('jsx-curly-brace-presence', rule, {
       output: '<MyComponent prop="< style: true >">foo</MyComponent>',
       errors: [{ messageId: 'unnecessaryCurly' }],
       options: ['never'],
+    },
+    {
+      code: `<App horror=<div /> />`,
+      output: `<App horror={<div />} />`,
+      errors: [{ messageId: 'missingCurly' }],
+      options: [{ props: 'always', children: 'always', propElementValues: 'always' }],
+      features: ['no-ts'],
+    },
+    {
+      code: `<App horror={<div />} />`,
+      output: `<App horror=<div /> />`,
+      errors: [{ messageId: 'unnecessaryCurly' }],
+      options: [{ props: 'never', children: 'never', propElementValues: 'never' }],
+      features: ['no-ts'],
     }
   )),
 });