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

Add support for running prettier on script in Vue Single File Components #70

Closed
wants to merge 4 commits into from

Conversation

jakub300
Copy link

Hey,
It is intended to be used with upcoming vue-eslint-parser v2 that is used by upcoming eslint-plugin-vue v4.
You can find examples of .vue files in GitLab or OnsenUI.

@not-an-aardvark
Copy link
Member

Thanks for the PR. However, I'm confused about why it's necessary to add special logic to detect whether vue-eslint-parser is being used (and why it's necessary to add special logic for vue in general). Could you clarify what problem this PR is trying to solve?

@jakub300
Copy link
Author

jakub300 commented Dec 7, 2017

Hey, sorry, I wasn't sure what to include in the description.

Let's start with example .vue file:

<template>
  <div id="app">
    <h1>{{ msg }}</h1>
  </div>
</template>

<script>
export default {
  name: 'app',
  data () {
    return {
      msg: 'Welcome to Your Vue.js App'
    }
  }
}
</script>

<style>
#app {

}
</style>

How to lint or run prettier on script in this file?
Due to the fact that those tags look like xml/html current approach to that is use of eslint-plugin-html. When it is used all prettier see when calling sourceCode.text is content of script tag.

But that will soon not be possible if one will be interested in using official vue eslint plugin/config. It will use custom parser that understands not only script but also template part of the file allowing creation of eslint rules enforcing style in html. When this parser is used sourceCode.text returns full content of vue file (not just script part). When this is sent to prettier, it is trying to parse it as jsx but obviously fails.

I was looking for relatively simple way to support running prettier on script in vue (in a universal, enforceable way, only other way is plugin for VS Code) and that lead me to playing with eslint-plugin-prettier and this PR.

@not-an-aardvark
Copy link
Member

I see, thanks for clarifying.

That said, I don't think it's a good idea for eslint-plugin-prettier to add special logic for vue. In my mind, there are two problems with doing that:

  • If eslint-plugin-prettier started adding special logic for Vue, then people might also want eslint-plugin-prettier to add support for other templating languages. This would cause eslint-plugin-prettier to become fairly complex, and it could also lead to problems if there were two supported template languages with conflicting syntax, because eslint-plugin-prettier might not know which semantics the user intended.
  • Conceptually, this doesn't seem ideal from a design perspective. I think of eslint-plugin-prettier as a bridge between eslint and prettier, and adding custom library-specific support seems like it's not directly related to that purpose.

I think it would be better if there was a solution for your problem that didn't involve making modifications to eslint-plugin-prettier.

I maintain eslint-rule-composer, which is a utility for extending the functionality of eslint rules. It's often useful for this type of problem (where one needs to have an eslint rule with slightly modified behavior from an existing rule). Unfortunately, it doesn't currently support transformations like this, where you would want to run the rule on a different piece of source text.

If we added a mapSourceCode method to eslint-rule-composer, then it would be possible to extend the behavior of eslint-plugin-prettier in a separate plugin (say, eslint-plugin-prettier-vue), without modifying eslint-plugin-prettier itself.

I'm imagining the API would look something like this, where mapSourceCode accepts a function which maps the SourceCode instance provided by ESLint to the SourceCode instance which should be accepted by the rule.

const ruleComposer = require("eslint-rule-composer");
const SourceCode = require("eslint").SourceCode;
const eslintPluginPrettierRule = require("eslint-plugin-prettier").rules.prettier;

const enhancedPrettierRule = ruleComposer.mapSourceCode(
  eslintPluginPrettierRule,
  sourceCode => {
    // do something vue-specific, such as extracting text from <script> tags
    const scriptTagText = extractScriptTagText(sourceCode.text);
    const scriptTagAst = extractScriptTagAst(sourceCode.ast);
    return new SourceCode(scriptTagText, scriptTagAst);
  }
);

// enhancedPrettierRule is now a new rule that preprocesses
// the AST and passes it to `eslint-plugin-prettier`

Does that sound reasonable to you? I would accept a PR on eslint-rule-composer adding an API like that.

@jakub300
Copy link
Author

Hey, thanks for long and comprehensive answer. The concept of eslint-rule-composer sounds interesting. Unfortunately recently I have less free time than a month ago. I will try to look into it at some point but no ETA.

@azz
Copy link
Member

azz commented Dec 27, 2017

In the next version of Prettier we'll support Vue components out-of-the-box, so this plugin should "just work" as-is.

@jakub300 jakub300 closed this Jan 9, 2018
@trainiac
Copy link

trainiac commented Jan 11, 2018

I've tried eslint-plugin-prettier with the latest prettier and found that it doesn't work. Perhaps this needs to be reopened? It doesnt work in the sense that eslint is still throwing errors on the template part of the vue file.

@azz
Copy link
Member

azz commented Jan 12, 2018

You'll still need at least eslint-plugin-html I think?

@jacobmllr95
Copy link

I have the same problems with eslint-plugin-prettier combined with eslint-plugin-vue. Each of the plugins works without the other. Adding eslint-plugin-html doesn't resolve the problem because it breaks eslint-plugin-vue features.

Here is my current .eslintrc:

{
  "extends": ["airbnb-base/legacy", "plugin:vue/recommended", "plugin:prettier/recommended"],
  "parserOptions": {
    "ecmaVersion": 8,
    "sourceType": "module",
    "ecmaFeatures": {
      "jsx": true,
      "experimentalObjectRestSpread": true
    }
  },
  "env": {
    "browser": true,
    "es6": true
  },
  "rules": {
    "camelcase": "off",
    "consistent-return": "off",
    "guard-for-in": "off",
    "no-extra-boolean-cast": "off",
    "no-multi-assign": "off",
    "no-nested-ternary": "off",
    "no-param-reassign": "off",
    "no-prototype-builtins": "off",
    "no-shadow": "off",
    "no-restricted-syntax": "off",
    "no-underscore-dangle": "off",
    "no-use-before-define": "off"
  }
}

@azz
Copy link
Member

azz commented Jan 12, 2018

@jackmu95 PR to fix that inbound 😄

Edit: #76

@jacobmllr95
Copy link

@azz Thanks for you investigations :)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants