Skip to content

Commit 7586915

Browse files
ljharbyannickcr
authored andcommitted
Fix jsx-curly-spacing alternative option
1 parent faaa823 commit 7586915

File tree

4 files changed

+106
-69
lines changed

4 files changed

+106
-69
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ Finally, enable all of the rules that you would like to use. Use [our preset](#
7575
"react/jsx-uses-react": "error",
7676
"react/jsx-uses-vars": "error",
7777
}
78-
```
78+
```
7979

8080
# List of supported rules
8181

docs/rules/jsx-curly-spacing.md

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -107,26 +107,34 @@ The following patterns are not warnings:
107107
<Hello name={ {firstname: 'John', lastname: 'Doe'} } />;
108108
```
109109

110-
#### Alternative
110+
#### Granular spacing controls
111111

112-
When setting the `alternative` option to `true` you must collapse the curly braces:
112+
You can specify an additional `spacing` property that is an object with the following possible values:
113113

114114
```json
115-
"jsx-curly-spacing": [2, "always", {"alternative": true}]
115+
"jsx-curly-spacing": [2, "always", {"spacing": {
116+
"objectLiterals": "never"
117+
}}]
116118
```
117119

118-
When `"always"` is used and `alternative` is `true`, the following pattern is not warnings:
120+
* `objectLiterals`: This controls different spacing requirements when the value inside the jsx curly braces is an object literal.
121+
122+
All spacing options accept either the string `"always"` or the string `"never"`. Note that the default value for all "spacing" options matches the first "always"/"never" option provided.
123+
124+
When `"always"` is used but `objectLiterals` is `"never"`, the following pattern is not considered a warning:
119125

120126
```js
121-
<App foo={{ bar: true, baz: true }} />;
127+
<App blah={ 3 } foo={{ bar: true, baz: true }} />;
122128
```
123129

124-
When `"always"` is used and `alternative` is `true`, the following pattern is considered warnings:
130+
When `"never"` is used and `objectLiterals` is `"always"`, the following pattern is not considered a warning:
125131

126132
```js
127-
<App foo={ {bar: true, baz: true} } />;
133+
<App blah={3} foo={ {bar: true, baz: true} } />;
128134
```
129135

136+
Please note that spacing of the object literal curly braces themselves is controlled by the built-in [`object-curly-spacing`](http://eslint.org/docs/rules/object-curly-spacing) rule.
137+
130138
## When Not To Use It
131139

132140
You can turn this rule off if you are not concerned with the consistency around the spacing inside of JSX attributes.

lib/rules/jsx-curly-spacing.js

Lines changed: 56 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,20 @@
1414
// Rule Definition
1515
// ------------------------------------------------------------------------------
1616

17+
var SPACING = {
18+
always: 'always',
19+
never: 'never'
20+
};
21+
var SPACING_VALUES = [SPACING.always, SPACING.never];
22+
1723
module.exports = function(context) {
1824

1925
var sourceCode = context.getSourceCode();
20-
var spaced = context.options[0] === 'always';
26+
var spaced = context.options[0] === SPACING.always;
2127
var multiline = context.options[1] ? context.options[1].allowMultiline : true;
22-
var alternative = context.options[1] ? context.options[1].alternative : false;
28+
var spacing = context.options[1] ? context.options[1].spacing || {} : {};
29+
var defaultSpacing = spaced ? SPACING.always : SPACING.never;
30+
var objectLiteralSpacing = spacing.objectLiterals || (spaced ? SPACING.always : SPACING.never);
2331

2432
// --------------------------------------------------------------------------
2533
// Helpers
@@ -151,57 +159,59 @@ module.exports = function(context) {
151159
var last = sourceCode.getLastToken(node);
152160
var second = context.getTokenAfter(first);
153161
var penultimate = sourceCode.getTokenBefore(last);
154-
var third = context.getTokenAfter(second);
155-
var antepenultimate = sourceCode.getTokenBefore(penultimate);
156-
157-
if (spaced) {
158-
if (!alternative) {
159-
if (!sourceCode.isSpaceBetweenTokens(first, second)) {
160-
reportRequiredBeginningSpace(node, first);
161-
} else if (!multiline && isMultiline(first, second)) {
162-
reportNoBeginningNewline(node, first);
163-
}
164-
165-
if (!sourceCode.isSpaceBetweenTokens(penultimate, last)) {
166-
reportRequiredEndingSpace(node, last);
167-
} else if (!multiline && isMultiline(penultimate, last)) {
168-
reportNoEndingNewline(node, last);
169-
}
170162

171-
// Object literal
172-
} else if (first.value === second.value) {
163+
var isObjectLiteral = first.value === second.value;
164+
if (isObjectLiteral) {
165+
if (objectLiteralSpacing === SPACING.never) {
173166
if (sourceCode.isSpaceBetweenTokens(first, second)) {
174167
reportNoBeginningSpace(node, first);
168+
} else if (!multiline && isMultiline(first, second)) {
169+
reportNoBeginningNewline(node, first);
175170
}
176171
if (sourceCode.isSpaceBetweenTokens(penultimate, last)) {
177172
reportNoEndingSpace(node, last);
173+
} else if (!multiline && isMultiline(penultimate, last)) {
174+
reportNoEndingNewline(node, last);
178175
}
179-
if (!sourceCode.isSpaceBetweenTokens(second, third)) {
180-
reportRequiredBeginningSpace(node, second);
181-
}
182-
if (!sourceCode.isSpaceBetweenTokens(antepenultimate, penultimate)) {
183-
reportRequiredEndingSpace(node, penultimate);
184-
}
185-
186-
} else {
176+
} else if (objectLiteralSpacing === SPACING.always) {
187177
if (!sourceCode.isSpaceBetweenTokens(first, second)) {
188178
reportRequiredBeginningSpace(node, first);
179+
} else if (!multiline && isMultiline(first, second)) {
180+
reportNoBeginningNewline(node, first);
189181
}
190182
if (!sourceCode.isSpaceBetweenTokens(penultimate, last)) {
191183
reportRequiredEndingSpace(node, last);
184+
} else if (!multiline && isMultiline(penultimate, last)) {
185+
reportNoEndingNewline(node, last);
192186
}
193187
}
188+
} else if (defaultSpacing === SPACING.always) {
189+
if (!sourceCode.isSpaceBetweenTokens(first, second)) {
190+
reportRequiredBeginningSpace(node, first);
191+
} else if (!multiline && isMultiline(first, second)) {
192+
reportNoBeginningNewline(node, first);
193+
}
194194

195-
return;
196-
}
197-
198-
// "never" setting if we get here.
199-
if (sourceCode.isSpaceBetweenTokens(first, second) && !(multiline && isMultiline(first, second))) {
200-
reportNoBeginningSpace(node, first);
201-
}
202-
203-
if (sourceCode.isSpaceBetweenTokens(penultimate, last) && !(multiline && isMultiline(penultimate, last))) {
204-
reportNoEndingSpace(node, last);
195+
if (!sourceCode.isSpaceBetweenTokens(penultimate, last)) {
196+
reportRequiredEndingSpace(node, last);
197+
} else if (!multiline && isMultiline(penultimate, last)) {
198+
reportNoEndingNewline(node, last);
199+
}
200+
} else if (defaultSpacing === SPACING.never) {
201+
if (isMultiline(first, second)) {
202+
if (!multiline) {
203+
reportNoBeginningNewline(node, first);
204+
}
205+
} else if (sourceCode.isSpaceBetweenTokens(first, second)) {
206+
reportNoBeginningSpace(node, first);
207+
}
208+
if (isMultiline(penultimate, last)) {
209+
if (!multiline) {
210+
reportNoEndingNewline(node, last);
211+
}
212+
} else if (sourceCode.isSpaceBetweenTokens(penultimate, last)) {
213+
reportNoEndingSpace(node, last);
214+
}
205215
}
206216
}
207217

@@ -216,15 +226,20 @@ module.exports = function(context) {
216226
};
217227

218228
module.exports.schema = [{
219-
enum: ['always', 'never']
229+
enum: SPACING_VALUES
220230
}, {
221231
type: 'object',
222232
properties: {
223233
allowMultiline: {
224234
type: 'boolean'
225235
},
226-
alternative: {
227-
type: 'boolean'
236+
spacing: {
237+
type: 'object',
238+
properties: {
239+
objectLiterals: {
240+
enum: SPACING_VALUES
241+
}
242+
}
228243
}
229244
},
230245
additionalProperties: false

tests/lib/rules/jsx-curly-spacing.js

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -84,19 +84,19 @@ ruleTester.run('jsx-curly-spacing', rule, {
8484
parserOptions: parserOptions
8585
}, {
8686
code: '<App foo={ bar } />;',
87-
options: ['always', {alternative: true}],
87+
options: ['always', {spacing: {}}],
8888
parserOptions: parserOptions
8989
}, {
9090
code: '<App foo={{ bar: true, baz: true }} />;',
91-
options: ['always', {alternative: true}],
91+
options: ['always', {spacing: {objectLiterals: 'never'}}],
9292
parserOptions: parserOptions
9393
}, {
9494
code: [
9595
'<App foo={',
9696
'bar',
9797
'} />;'
9898
].join('\n'),
99-
options: ['always', {alternative: true}],
99+
options: ['always', {allowMultiline: true}],
100100
parserOptions: parserOptions
101101
}, {
102102
code: '<App {...bar} />;',
@@ -178,6 +178,10 @@ ruleTester.run('jsx-curly-spacing', rule, {
178178
code: '<App foo={bar/* comment */} {...baz/* comment */} />;',
179179
options: ['never'],
180180
parserOptions: parserOptions
181+
}, {
182+
code: '<App foo={3} bar={ {a: 2} } />',
183+
options: ['never', {spacing: {objectLiterals: 'always'}}],
184+
parserOptions: parserOptions
181185
}],
182186

183187
invalid: [{
@@ -261,9 +265,9 @@ ruleTester.run('jsx-curly-spacing', rule, {
261265
output: '<App foo={bar} />;',
262266
options: ['never', {allowMultiline: false}],
263267
errors: [{
264-
message: 'There should be no space after \'{\''
268+
message: 'There should be no newline after \'{\''
265269
}, {
266-
message: 'There should be no space before \'}\''
270+
message: 'There should be no newline before \'}\''
267271
}],
268272
parserOptions: parserOptions
269273
}, {
@@ -283,7 +287,7 @@ ruleTester.run('jsx-curly-spacing', rule, {
283287
}, {
284288
code: '<App foo={bar} />;',
285289
output: '<App foo={ bar } />;',
286-
options: ['always', {alternative: true}],
290+
options: ['always', {spacing: {}}],
287291
errors: [{
288292
message: 'A space is required after \'{\''
289293
}, {
@@ -293,29 +297,25 @@ ruleTester.run('jsx-curly-spacing', rule, {
293297
}, {
294298
code: '<App foo={ bar} />;',
295299
output: '<App foo={ bar } />;',
296-
options: ['always', {alternative: true}],
300+
options: ['always', {spacing: {}}],
297301
errors: [{
298302
message: 'A space is required before \'}\''
299303
}],
300304
parserOptions: parserOptions
301305
}, {
302306
code: '<App foo={bar } />;',
303307
output: '<App foo={ bar } />;',
304-
options: ['always', {alternative: true}],
308+
options: ['always', {spacing: {}}],
305309
errors: [{
306310
message: 'A space is required after \'{\''
307311
}],
308312
parserOptions: parserOptions
309313
}, {
310314
code: '<App foo={ {bar: true, baz: true} } />;',
311-
output: '<App foo={{ bar: true, baz: true }} />;',
312-
options: ['always', {alternative: true}],
315+
output: '<App foo={{bar: true, baz: true}} />;',
316+
options: ['always', {spacing: {objectLiterals: 'never'}}],
313317
errors: [{
314318
message: 'There should be no space after \'{\''
315-
}, {
316-
message: 'A space is required after \'{\''
317-
}, {
318-
message: 'A space is required before \'}\''
319319
}, {
320320
message: 'There should be no space before \'}\''
321321
}],
@@ -401,9 +401,9 @@ ruleTester.run('jsx-curly-spacing', rule, {
401401
output: '<App {...bar} />;',
402402
options: ['never', {allowMultiline: false}],
403403
errors: [{
404-
message: 'There should be no space after \'{\''
404+
message: 'There should be no newline after \'{\''
405405
}, {
406-
message: 'There should be no space before \'}\''
406+
message: 'There should be no newline before \'}\''
407407
}],
408408
parserOptions: parserOptions
409409
}, {
@@ -527,13 +527,13 @@ ruleTester.run('jsx-curly-spacing', rule, {
527527
output: '<App foo={bar} {...baz} />;',
528528
options: ['never', {allowMultiline: false}],
529529
errors: [{
530-
message: 'There should be no space after \'{\''
530+
message: 'There should be no newline after \'{\''
531531
}, {
532-
message: 'There should be no space before \'}\''
532+
message: 'There should be no newline before \'}\''
533533
}, {
534-
message: 'There should be no space after \'{\''
534+
message: 'There should be no newline after \'{\''
535535
}, {
536-
message: 'There should be no space before \'}\''
536+
message: 'There should be no newline before \'}\''
537537
}],
538538
parserOptions: parserOptions
539539
}, {
@@ -556,5 +556,19 @@ ruleTester.run('jsx-curly-spacing', rule, {
556556
message: 'There should be no newline before \'}\''
557557
}],
558558
parserOptions: parserOptions
559+
}, {
560+
code: '<App foo={ 3 } bar={{a: 2}} />',
561+
output: '<App foo={3} bar={ {a: 2} } />',
562+
options: ['never', {spacing: {objectLiterals: 'always'}}],
563+
errors: [{
564+
message: 'There should be no space after \'{\''
565+
}, {
566+
message: 'There should be no space before \'}\''
567+
}, {
568+
message: 'A space is required after \'{\''
569+
}, {
570+
message: 'A space is required before \'}\''
571+
}],
572+
parserOptions: parserOptions
559573
}]
560574
});

0 commit comments

Comments
 (0)