Skip to content

Request to allow vue component js to optionally start at an indentation level of 1 when wrapped in <script></script> #118

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

Closed
prurigro opened this issue Jul 31, 2017 · 25 comments · Fixed by #309

Comments

@prurigro
Copy link

The version of ESLint you are using: v4.3.0

The version of eslint-plugin-vue you are using: 3.8.0

The problem you want to solve: Javascript in a vue component expects an initial indentation level of 0 despite being wrapped in <script></script>. The following currently reports an error:

<script>
    export default {

    };
</script>

Your take on the correct solution to problem: When modifying a vue component, the linter should optionally allow for an initial indentation level of 1 when wrapped in <script></script>.

The eslint-plugin-html plugin solves the indentation level issue for html files, and works nicely with eslint to only lint the javascript portion of vue components. Ideally eslint-plugin-vue could work in a similar way while also linting the template portion of the component (as it currently does).

Thanks!

@iGarym
Copy link

iGarym commented Jul 31, 2017

Same proposition.

eslint-plugin-html has a nice rule:

"settings": {
  "html/indent": "+2",
}

@armano2
Copy link
Contributor

armano2 commented Jul 31, 2017

@iGarym #46 we are waiting for new parser and we can start working a bit more on html/template part of eslint

@prurigro
Copy link
Author

@armano2 That seems to be talking about the indentation level of html elements in the <template></template> portion of the component, would it also be required to allow the contents of the <script></script> tag to start at one indentation level higher? I suppose if it's a matter of either forcing the increased level or not then a separate plugin could be written to be used in conjunction with this one until the updated parser exists; being optional does seem appropriate as I've noticed some examples of people keeping the contents of the script tag flush with the tag.

@armano2
Copy link
Contributor

armano2 commented Jul 31, 2017

@prurigro it has to be done on parser level -> right now code is parsed first by html parser after that part of code is passed to espree (eslint) and they are dealing with it, adding this feature has to be done before passing code into espree.

its possbile, but i don't think so that there is going to be anyone to do that before new parser will be released, #116.

@mysticatea can you confirm/deny this?

@mysticatea
Copy link
Member

Thank you for this issue.

I confirmed. As far as I know, the indent rule of eslint@3 is no problem in this case, but eslint@4's reports errors. We should address this problem in any way.

For now, you can use indent-legacy rule which has the same behavior as the indent rule of eslint@3 as a workaround.

{
    "indent": "off",
    "indent-legacy": ["error", 4]
}

@michalsnik michalsnik added this to the Official release milestone Aug 3, 2017
@mysticatea
Copy link
Member

In #145, I'm making almost full-spec indent rule eventually because v-on directive can have statements. (I had thought it needs only about expressions at first.)
So I think that I can make vue/script-indent rule which shares almost code with vue/html-indent, then it has the option to specify the base indent.

@michalsnik
Copy link
Member

#145 is now merged and we could work on this, but replacing core rule with tailored one seem not like the best solution to me. Can we perhaps somehow alter the parsed AST so that the core rule sees different indentation @mysticatea ?

@mysticatea
Copy link
Member

mysticatea commented Nov 16, 2017

No, it cannot do in vue-eslint-parser side because custom parsers cannot modify the linting result to back warning/fixing locations after linting.

Plugin preprocessor can modify source code text and the linting result. The plugins which have preprocessor can support autofix since [email protected]. However, I'm not sure how the preprocessor should behave in the following case:

<script>
    aaa()
    bbb()
ccc()
    ddd()
</script>

I think it cannot be unindented because ccc() does not have its indentation. As the result, indent rule warns aaa(), bbb(), and ddd() as unexpected. On the other hand, if preprocessor unindents the lines except ccc(), indent rule overlooks the ccc() line.

@michalsnik
Copy link
Member

I'm not sure I understand you @mysticatea I think we should detect indentation of the first line and treat it as the base indentation, so the ccc should only be reported as badly indented line. Can we do something about it?

@mysticatea
Copy link
Member

the ccc should only be reported as badly indented line.

You are right. But I think that we cannot make AST like that the core indent rule reports the ccc() line. My above comment is the explanation of that.

@michalsnik
Copy link
Member

Ah now I get it @mysticatea. Makes sense, though I'm wondering how did guys figured it out in eslint-plugin-html. Do you think that having a dedicated rule for script indentation (even copy of indent-legacy) is the legitimate solution? If yes - then let's do this. We can obviously iterate over it, but we could already ship a full featured 4.0-beta.1 after it's done. What do you think? cc @chrisvfritz

@chrisvfritz
Copy link
Contributor

I think it makes sense as a dedicated rule, because whether a project wants to start at indentation 0 or 1, they'll probably want to enforce consistency. I think ideally, we'd also provide the ability to configure each tag type individually, because the most common convention I see is:

  • 1 for <template>
  • 0 for <script>
  • 0 for <style>

I'd also suggest that as the default, with the rule in the recommended category.

@michalsnik
Copy link
Member

Okay, that being said I don't think it's something that blocks us from releasing 4.0. We can work on this simultaneously and for now put information in readme that if someone wants to have extra indent in script, he might just use indent-legacy rule from eslint.

@michalsnik
Copy link
Member

Especially if in the end we're going to put this in recommended category anyway ☝️

@chrisvfritz
Copy link
Contributor

Agreed. 👍

@michalsnik michalsnik removed this from the v4.1.0 milestone Nov 26, 2017
@tronjs
Copy link

tronjs commented Dec 28, 2017

Hi, just question, whether this issue was fixed yet?

@MicroDroid
Copy link

@tronjs It would have been closed if it was.

michalsnik pushed a commit that referenced this issue Jan 7, 2018
* Chore: extract common part of indent

* Chore: refactor tests of html-indent

* New: vue/script-indent (fixes #118)

* Chore: upgrade deps

* Chore: fix test script

- To dedupe deps

* Chore: revive lost tests

* Update: add alignAttributesVertically

* change category

* add comments
@MicroDroid
Copy link

MicroDroid commented Jan 11, 2018

It's still unclear how to use this new vue/script-indent rule. I've added something like this to my .eslintrc:

"vue/script-indent": [
    "error",
    "tab",
    {"baseIndent": 1}
],

I still got warnings from the indent rule. I removed the indent rule, now other non-.Vue files don't get indentation linting properly. Also vue/script-indent is also applying its indentation linting on non-.Vue files like .js files, causing my normal .js files to have indentation starting at 1

@mysticatea
Copy link
Member

@MicroDroid Would you open new issue with details?

@MicroDroid
Copy link

@mysticatea I just tried again and removed the indent rule and kept the vue/script-indent one, everything worked fine. I have no idea why it did not work earlier.

@mitar
Copy link

mitar commented Jan 11, 2018

But the vue/script-indent has much less configuration options than indent? Is vue/script-indent a superset of indent + base indent feature? Because I like the strict indent nature.

Also, it seems you cannot use vue/script-indent just for <script> tags, while keeping indent for all other code?

@MicroDroid
Copy link

MicroDroid commented Jan 11, 2018

I actually ended up with #344 (Posting here so people having the same issue in this conversation can link to there)

@manico
Copy link

manico commented Mar 2, 2018

Is there a solution for this?
I ended up removing eslint-plugin-vue and reverting to eslint-plugin-html. :(

@MicroDroid
Copy link

@manico This has been fixed long ago. Update the package and use config such as one in #344 or simply read docs.

@mitar
Copy link

mitar commented Mar 3, 2018

I opened #418 for more compatibility with indent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants