Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix wrap-multilines rule #728

Merged
merged 1 commit into from
Jul 31, 2016
Merged

Conversation

akozhemiakin
Copy link
Contributor

Wrap multilines was broken after switching to new eslint rule format.

@ljharb
Copy link
Member

ljharb commented Jul 31, 2016

Thanks! Can you add a test that would have caught this bug?

@akozhemiakin
Copy link
Contributor Author

@ljharb I thought about adding a test but it is a deprecated rule that uses jsx-wrap-multilines rule under the hood.

So I'm not sure how to test it properly. Should we just copy this test https://github.com/yannickcr/eslint-plugin-react/blob/master/tests/lib/rules/jsx-wrap-multilines.js? Or should we create separate test with a single random valid code snippet to just test that it works?

@ljharb
Copy link
Member

ljharb commented Jul 31, 2016

I think that as long as we're keeping the deprecated test we should keep the deprecated test file - so yeah, just duplicating https://github.com/yannickcr/eslint-plugin-react/blob/master/tests/lib/rules/jsx-wrap-multilines.js is probably good.

Wrap multilines was broken after switching to new eslint rule format.
Also in this commit wrap-multilines was aligned with
jsx-wrap-multilines: "fixed: 'code'" was added to its meta.
@akozhemiakin akozhemiakin force-pushed the fix-wrap-multilines branch from ffab8c1 to 99f5eac Compare July 31, 2016 01:19
@akozhemiakin
Copy link
Contributor Author

Yeah, it is definitely a good approach to duplicate that test file :) It revealed another bug with missing meta property (fixable: 'code'). I fixed it and added the test.

@ljharb ljharb added the bug label Jul 31, 2016
@ljharb ljharb merged commit 99f5eac into jsx-eslint:master Jul 31, 2016
@ljharb
Copy link
Member

ljharb commented Jul 31, 2016

Thanks!

@akozhemiakin akozhemiakin deleted the fix-wrap-multilines branch July 31, 2016 03:06
ljharb added a commit that referenced this pull request Jul 31, 2016
This reverts commit 2f7a462 per 2f7a462#commitcomment-18463332.

It also fixes a bug in `no-comment-textnodes` related to #728, and restores the tests for `no-comment-textnodes` from #689.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants