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

Fix jsx-curly-spacing: spacing around spread operator #610

Closed
wants to merge 2 commits into from
Closed

Fix jsx-curly-spacing: spacing around spread operator #610

wants to merge 2 commits into from

Conversation

gitim
Copy link
Contributor

@gitim gitim commented May 22, 2016

Fixes #606
Added two type of tests:

  • creating jsx elements using only JSXSpreadAttribute syntax
  • creating jsx elements using both JSXSAttribute and JSXSpreadAttribute syntax

var first = context.getFirstToken(node);
var last = sourceCode.getLastToken(node);
var second = context.getTokenAfter(first);
var penultimate = sourceCode.getTokenBefore(last);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just put these declarations inside the function to reduce copy pasting

@coveralls
Copy link

coveralls commented May 22, 2016

Coverage Status

Coverage decreased (-0.002%) to 96.767% when pulling cdd2b17 on gitim:fix-jsx-curly-spacing into 82b3aa9 on yannickcr:master.

code: '<App foo={bar/* comment */} {...baz/* comment */} />;',
options: ['never'],
parserOptions: parserOptions,
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.

why do you need the babel-eslint parser for jsx object spread? that should only be needed for JS object spread.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

hm, i wouldn't expect that one to need it either

Copy link
Contributor Author

@gitim gitim May 23, 2016

Choose a reason for hiding this comment

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

this test case was added in #584, and it seems that the issue wasn't related to babel-eslint parser, so I think it is safe to remove them both

@coveralls
Copy link

coveralls commented May 23, 2016

Coverage Status

Coverage decreased (-0.002%) to 96.767% when pulling 7e4f0d8 on gitim:fix-jsx-curly-spacing into 82b3aa9 on yannickcr:master.

@yannickcr
Copy link
Member

Merged, thanks!

@yannickcr yannickcr closed this Jun 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants