Skip to content

Commit

Permalink
[fix] Allow whitespace jsx container in jsx-curly-brace-presence (fixes
Browse files Browse the repository at this point in the history
…: #1717)
  • Loading branch information
sharmilajesupaul committed Mar 12, 2018
1 parent 14b8d01 commit e907b67
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 4 deletions.
8 changes: 7 additions & 1 deletion docs/rules/jsx-curly-brace-presence.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,12 +139,18 @@ will warned and fixed to:

* If the rule is set to get rid of unnecessary curly braces(JSX expression) and there are characters that need to be escaped in its JSX form, such as quote characters, [forbidden JSX text characters](https://facebook.github.io/jsx/), escaped characters and anything that looks like HTML entity names, the code will not be warned because the fix may make the code less readable.

The following pattern will **not** be given a warning even if `'never'` is passed.
The following patterns will **not** be given a warning even if `'never'` is passed.

```jsx
<Color text={"\u00a0"} />
<App>{"Hello \u00b7 world"}</App>;
<style type="text/css">{'.main { margin-top: 0; }'}</style>;
/**
* there's no way to inject a whitespace into jsx without a container so this
* will always be allowed.
*/
<App>{' '}</App>
<App>{' '}</App>
```

## When Not To Use It
Expand Down
30 changes: 27 additions & 3 deletions lib/rules/jsx-curly-brace-presence.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,14 @@ module.exports = {
);
}

function containsWhitespaceExpression(child) {
if (child.type === 'JSXExpressionContainer') {
const value = child.expression.value;
return value ? !(/\S/.test(value)) : false;
}
return false;
}

/**
* Report and fix an unnecessary curly brace violation on a node
* @param {ASTNode} node - The AST node with an unnecessary JSX expression
Expand Down Expand Up @@ -204,11 +212,27 @@ module.exports = {
return false;
}

if (
parent.children
&& parent.children.length === 1
&& containsWhitespaceExpression(parent.children[0])
) {
return false;
}

return areRuleConditionsSatisfied(parentType, config, OPTION_NEVER);
}

function shouldCheckForMissingCurly(parentType, config) {
return areRuleConditionsSatisfied(parentType, config, OPTION_ALWAYS);
function shouldCheckForMissingCurly(parent, config) {
if (
parent.children
&& parent.children.length === 1
&& containsWhitespaceExpression(parent.children[0])
) {
return false;
}

return areRuleConditionsSatisfied(parent.type, config, OPTION_ALWAYS);
}

// --------------------------------------------------------------------------
Expand All @@ -223,7 +247,7 @@ module.exports = {
},

Literal: node => {
if (shouldCheckForMissingCurly(node.parent.type, userConfig)) {
if (shouldCheckForMissingCurly(node.parent, userConfig)) {
reportMissingCurly(node);
}
}
Expand Down
26 changes: 26 additions & 0 deletions tests/lib/rules/jsx-curly-brace-presence.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,32 @@ ruleTester.run('jsx-curly-brace-presence', rule, {
code: '<App {...props}>foo</App>',
options: [{props: 'never'}]
},
/*
* There is no way to inject the space into JSX without an expression container
* so this format should always be allowed regardless of the `children` option.
*/
{
code: '<App>{\' \'}</App>'
},
{
code: '<App>{\' \'}</App>'
},
{
code: '<App>{\' \'}</App>',
options: [{children: 'never'}]
},
{
code: '<App>{\' \'}</App>',
options: [{children: 'never'}]
},
{
code: '<App>{\' \'}</App>',
options: [{children: 'always'}]
},
{
code: '<App>{\' \'}</App>',
options: [{children: 'always'}]
},
{
code: '<App {...props}>foo</App>',
options: [{props: 'always'}]
Expand Down

0 comments on commit e907b67

Please sign in to comment.