BenoitZugmeyer / eslint-plugin-html

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

eslint-disable-* Has No Effect on Single Line Script #49

Closed niedzielski closed 7 years ago

niedzielski commented 7 years ago

Thank you for the great plugin! I'm a new user and found a very minor issue. eslint-disable-* comments don't work on script opening and closing tags that appear on the same line:

<script>SystemJS.import('/index.js') // eslint-disable-line no-undef</script>

The following alternatives all work fine:

<script>
  // eslint-disable-next-line no-undef
  SystemJS.import('/index.js')
</script>

<script>
  SystemJS.import('/index.js') // eslint-disable-line no-undef
</script>

<script>SystemJS.import('/index.js') // eslint-disable-line no-undef
</script>
aftdotleo commented 7 years ago

I've passed linter with:

/*eslint-disable*/
// your script ...
/*eslint-enable*/
</script>
BenoitZugmeyer commented 7 years ago

Thank you very much for the report. I'm AFK right know, I will fix this next week!

BenoitZugmeyer commented 7 years ago

Sorry for the wait but I don't really know what to do with this issue. Currently, all HTML parts are replaced by a /* HTML */ comment before being fed to ESLint. So

<script>SystemJS.import('/index.js') // eslint-disable-line no-undef</script>

will be transformed to

/* HTML */SystemJS.import('/index.js') // eslint-disable-line no-undef/* HTML */

and ESLint doesn't handles the disable-line comment formated like that. A solution would be to remove the /* HTML */ comment completely, but that would be a breaking change. As it is a minor issue, I don't think fixing this is urgent, so let's wait if those comments are causing other issues before moving on the subject.

niedzielski commented 7 years ago

Thank you @BenoitZugmeyer for your nice explanation! Perhaps another idea would be:

/* HTML */SystemJS.import('/index.js')/* HTML */ // eslint-disable-line no-undef

But that kind of defeats the purpose of the HTML block comments :/ If the block comments can't be removed in the long term, maybe this issue can just be a known limitation mentioned in the readme. Thanks again!

BenoitZugmeyer commented 7 years ago

I released a beta version of eslint-plugin-html, you can test it with npm install eslint-plugin-html@next. I'll release a final version after eslint 4 is released.

Changes are listed here.

niedzielski commented 7 years ago

@BenoitZugmeyer, this fix works in my project. Thank you!