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

[Update] Make vue/no-shared-component-data fixable #278

Merged
merged 2 commits into from
Dec 3, 2017

Conversation

mysticatea
Copy link
Member

This PR makes vue/no-shared-component-data fixable. The autofix wraps the data property's value by a function expression.

At first glance, the unit tests make wrong indentations, but this is intentional. This strategy is same as core rules; one rule does one thing. A combination of rules will make correct autofix.

image

let last = sourceCode.getLastToken(p.value)

// If the value enclosed by parentheses, update the 'first' and 'last' by the parentheses.
let t = null
Copy link
Member

Choose a reason for hiding this comment

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

I think we should use more descriptive variables, t and u doesn't mean much.

// If the value enclosed by parentheses, update the 'first' and 'last' by the parentheses.
let t = null
let u = null
while (isOpenParen(t = sourceCode.getTokenBefore(first)) && isCloseParen(u = sourceCode.getTokenAfter(last))) {
Copy link
Member

Choose a reason for hiding this comment

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

I think we should avoid assigning values in function params, it's kind of confusing. Please simplify this part and wrap too long line.

// See: https://eslint.org/blog/2017/06/eslint-v4.1.0-released#applying-multiple-autofixes-simultaneously
const range = [first.range[0], last.range[1]]
const valueText = sourceCode.text.slice(range[0], range[1])
const replacement = `function() {\nreturn ${valueText};\n}`
Copy link
Member

Choose a reason for hiding this comment

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

Can't we replace it with data() {} instead of a function?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that a function expression is better to keep the rule simple.
If we want to fix to a method shorthand, it needs extra check the object-shorthand rule is doing. I.e., if comments exist between the key and the value, the rule should not autofix it to avoid removing the comments.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, alright. There is already object-shorthand core rule, that is fixable, so it should take care of it :) I forgot about it.

@michalsnik michalsnik changed the title Update: make vue/no-shared-component-data fixable [Update] Make vue/no-shared-component-data fixable Dec 2, 2017
@mysticatea
Copy link
Member Author

Updated about t/u.

@michalsnik
Copy link
Member

Good job @mysticatea 🙌

@michalsnik michalsnik merged commit 887045a into master Dec 3, 2017
@michalsnik michalsnik deleted the no-shared-component-data/fixable branch December 3, 2017 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants