Skip to content
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

Allow for stricter no-literals rule #1202

Merged
merged 8 commits into from Jul 2, 2017
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
43 changes: 39 additions & 4 deletions docs/rules/jsx-no-literals.md
@@ -1,11 +1,12 @@
# Prevent usage of unwrapped JSX strings (react/jsx-no-literals)
# Prevent usage of string literals in JSX (react/jsx-no-literals)

In JSX when using a literal string you can wrap it in a JSX container `{'TEXT'}`.
This rules requires that you wrap all literal strings.
Prevents any odd artifacts of highlighters if your unwrapped string contains an enclosing character like `'` in contractions and enforces consistency.
There are a couple of scenarios where you want to avoid string literals in JSX. Either to enforce consistency and reducing strange behaviour, or for enforcing that literals aren't kept in JSX so they can be translated.

## Rule Details

In JSX when using a literal string you can wrap it in a JSX container `{'TEXT'}`. This rules by default requires that you wrap all literal strings.
Prevents any odd artifacts of highlighters if your unwrapped string contains an enclosing character like `'` in contractions and enforces consistency.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ftr tho, this shouldn't ever happen, since straight quotes are typographically incorrect - and this would be a bug in the highlighter. I'm not sure this is worth mentioning.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the original text, essentially, with some modifications because of the different structure. If what you're saying is true, then this rule has/had no purpose.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It still has many purposes; but dealing with quotes isn't one of them imo :-)

Fine to leave the text as-is.


The following patterns are considered warnings:

```jsx
Expand All @@ -18,6 +19,40 @@ The following patterns are not considered warnings:
var Hello = <div>{'test'}</div>;
```

### Options

There is only one option:

* `no-strings` - Enforces no string literals used as children, wrapped or unwrapped.

To use, you can specify like the following:

```json
"react/jsx-no-literals": ["no-strings"]
```

In this configuration, the following are considered warnings:

```jsx
var Hello = <div>test</div>;
```

```jsx
var Hello = <div>{'test'}</div>;
```

The following are not considered warnings:

```jsx
// When using something like `react-intl`
var Hello = <div><Text {...message} /></div>
```

```jsx
// When using something similar to Rails translations
var Hello = <div>{translate('my.translation.key')}</div>
```

## When Not To Use It

If you do not want to enforce any style JSX literals, then you can disable this rule.
32 changes: 24 additions & 8 deletions lib/rules/jsx-no-literals.js
@@ -1,13 +1,16 @@
/**
* @fileoverview Prevent using string literals in React component definition
* @author Caleb Morris
* @author David Buchan-Swanson
*/
'use strict';

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------

var NO_STRINGS = 'no-strings';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const


module.exports = {
meta: {
docs: {
Expand All @@ -17,6 +20,8 @@ module.exports = {
},

schema: [{
enum: [NO_STRINGS]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than add a string option, let's add this to the options object, for future extensibility.

}, {
type: 'object',
properties: {},
additionalProperties: false
Expand All @@ -25,27 +30,38 @@ module.exports = {

create: function(context) {

const isNoStrings = context.options[0] === NO_STRINGS;

const message = isNoStrings ?
'Strings not allowed in JSX files' :
'Missing JSX expression container around literal string';

function reportLiteralNode(node) {
context.report({
node: node,
message: 'Missing JSX expression container around literal string'
message: message
});
}

function getValidation(node) {
const standard = !/^[\s]+$/.test(node.value) &&
node.parent &&
node.parent.type.indexOf('JSX') !== -1 &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will node.parent always have a type property that's a string or an array?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the existing validation. I just removed part of it if you enable the option.

node.parent.type !== 'JSXAttribute';
if (isNoStrings) {
return standard;
}
return standard && node.parent.type !== 'JSXExpressionContainer';
}

// --------------------------------------------------------------------------
// Public
// --------------------------------------------------------------------------

return {

Literal: function(node) {
if (
!/^[\s]+$/.test(node.value) &&
node.parent &&
node.parent.type !== 'JSXExpressionContainer' &&
node.parent.type !== 'JSXAttribute' &&
node.parent.type.indexOf('JSX') !== -1
) {
if (getValidation(node)) {
reportLiteralNode(node);
}
}
Expand Down
35 changes: 35 additions & 0 deletions tests/lib/rules/jsx-no-literals.js
@@ -1,6 +1,7 @@
/**
* @fileoverview Prevent using unwrapped literals in a React component definition
* @author Caleb morris
* @author David Buchan-Swanson
*/
'use strict';

Expand Down Expand Up @@ -109,6 +110,22 @@ ruleTester.run('jsx-no-literals', rule, {
'</Foo>'
].join('\n'),
parser: 'babel-eslint'
}, {
code: `
<Foo bar="test">
{intl.formatText(message)}
</Foo>
`,
parser: 'babel-eslint',
options: ['no-strings']
}, {
code: `
<Foo bar="test">
{translate('my.translate.key')}
</Foo>
`,
parser: 'babel-eslint',
options: ['no-strings']
}
],

Expand Down Expand Up @@ -202,6 +219,24 @@ ruleTester.run('jsx-no-literals', rule, {
].join('\n'),
parser: 'babel-eslint',
errors: [{message: 'Missing JSX expression container around literal string'}]
}, {
code: `
<Foo bar="test">
{'Test'}
</Foo>
`,
parser: 'babel-eslint',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add identical test cases that use the default parser.

options: ['no-strings'],
errors: [{message: 'Strings not allowed in JSX files'}]
}, {
code: `
<Foo bar="test">
Test
</Foo>
`,
parser: 'babel-eslint',
options: ['no-strings'],
errors: [{message: 'Strings not allowed in JSX files'}]
}
]
});