BenoitZugmeyer / eslint-plugin-html

An ESLint plugin to extract and lint scripts from HTML files.
ISC License
430 stars 51 forks source link

Enable --fix flag? #23

Closed wesbos closed 7 years ago

wesbos commented 8 years ago

Hey - was wondering how to get the --fix flag to work with this plugin?

Here is a quick video detailing my issue: http://wes.io/gM09

wesbos commented 8 years ago

Ah I see how this plugin works, it strips out the script and runs it against eslint, and eslint won't fix piped in text, only files :(

BenoitZugmeyer commented 8 years ago

Well, it works as a ESLint processor, and ESLint does not fix files transformed by a processor. There is an open issue eslint/eslint#5121 to discuss the possibility of doing so, and I guess this is a somewhat wanted feature by the devs too (see this comment for example), but right now this isn't feasible.

I tried to raise my voice to enhance the processor APIs but without any success for now.

I'll keep this issue open as this is a thing I'd like to have too (and I'm sure it will be doable sooner or later).

wesbos commented 8 years ago

Awesome - thanks for the info and your hard work on this :)

dashmug commented 8 years ago

Will it be feasible if we extract the js inside the <script>, write it to a temporary file, run eslint --fix on that file, and inject the fixed js back to the original html file?

skyrpex commented 8 years ago

Some eslint rules depend on the current path (for example, the eslint-plugin-import rules)... so, not sure if that would work.

skyrpex commented 8 years ago

Unless you can pass or overwrite the file path to ESLint.

BenoitZugmeyer commented 8 years ago

It would be feasible, yes, but not as a ESLint plugin. It should be a wrapper around ESLint and thus, a different project than this one. It could share some code though.

BenoitZugmeyer commented 8 years ago

Unless you can pass or overwrite the file path to ESLint.

I would use the --stdin-filename flag to do this.

yisibl commented 7 years ago

Try this plugin? https://github.com/lkiarest/eslint-plugin-vuefix

BenoitZugmeyer commented 7 years ago

@yisibl Based on ESLint source code, this plugin is unlikely to work: if a processor is used, ESLint will never populate the fixedResult variable and thus never define the result.output property used to write the fixed file.

DanHodges commented 7 years ago

If it helps anyone, I wrote a script to fix a large codebase of .vue files. It's kinda rough, but gets the job done- https://gist.github.com/DanHodges/33a880466b30274ba9e2e403c75133a5.

BenoitZugmeyer commented 7 years ago

Hi everyone,

I commited the final feature I wanted to see in the v2 of this plugin: the long awaited --fix support. I have written basic tests but sadly I have no longer access to a large number of HTML files containing script tags like I used to, so I'll need your help to let me know if it works for you before I release the new version. Just upgrade the plugin with npm install 'BenoitZugmeyer/eslint-plugin-html#v2' and try eslint --fix on some of your html files.

DanHodges commented 7 years ago

@BenoitZugmeyer It seemed to fix the errors, but it also added 10 lines of whitespace to the bottom of the file, directly after the closing </script> I'm using .vue files with vueify.

It seems to add an additional 10 lines with every subsequent time I run the command.

I've run --fix on several files, and this behavior seems to be consistent, even on files where no errors are fixed.

Thanks for working on this. Let me know what I can do to help.

BenoitZugmeyer commented 7 years ago

@DanHodges thank you for the fast feedback! vueify is only used to build the JS for the browser, I don't think it interfers with eslint? did you enable the rule eol-last? could you pinpoint the rule that is adding those new lines by disabling all rules and re-enabling one by one or something like that?

DanHodges commented 7 years ago

@BenoitZugmeyer The convention with vueify is save the files as .vue and format files like this-

<style>
  .red {
    color: #f00;
  }
</style>

<template>
  <h1 class="red">{{msg}}</h1>
</template>

<script>
export default {
  data () {
    return {
      msg: 'Hello world!'
    }
  }
}
</script>

That's the only reason I mentioned it. I agree, it shouldn't interfere with eslint. I'm using the airbnb style guide ("extends": "airbnb-base"). My only modifications are for console logs and function names.

Let me experiment with rules and if that changes anything 😄

BenoitZugmeyer commented 7 years ago

Ok after trying with the provided snippet + airbnb-base, I found out that it was indeed due to the eol-last rule. I made an issue #45 to fix this.

DanHodges commented 7 years ago

On my fairly large codebase of .vue and .js files, fix seems to be working great 👏

BenoitZugmeyer commented 7 years ago

Glad to hear it!

BenoitZugmeyer commented 7 years ago

v2.0.0 released, let me know if you've got any issue.