1230776 - Support ESLint in HTML files

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Kris Maglione [:kmag]]

+++ This bug was initially created as a clone of Bug #1229858 +++

This would be very useful for mochitests.

Comment 1

[Kris Maglione [:kmag]]

Attachment: MozReview Request: Bug 1230776: Support scripts from HTML files in eslint. r?gps

Bug 1230776: Support scripts from HTML files in eslint. r?Mossop

Comment 2

[Kris Maglione [:kmag]]

Comment on attachment 8696279 [details]
MozReview Request: Bug 1230776: Support scripts from HTML files in eslint. r?gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27303/diff/1-2/

Comment 3

[Mark Banner (:standard8)]

Note: using this add-on actually causes eslint within Sublime to be broken:

https://github.com/roadhump/SublimeLinter-eslint/issues/87
https://github.com/roadhump/SublimeLinter-eslint/issues/88
https://github.com/roadhump/SublimeLinter-eslint/issues/99 -> https://github.com/SublimeLinter/SublimeLinter3/issues/356

Whilst we might not want to block on these, it'd be good to see if we can push them forward to fix them for our developers.

Comment 4

[Kris Maglione [:kmag]]

Hm. Breaking Sublime is probably a non-starter...

Maybe we should enable it with the --plugin command line flag rather than in the global config until that's fixed.

Comment 5

[Kris Maglione [:kmag]]

Comment on attachment 8696279 [details]
MozReview Request: Bug 1230776: Support scripts from HTML files in eslint. r?gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27303/diff/2-3/

Comment 6

[Kris Maglione [:kmag]]

Switching to r?gps since the mach_commands.py changes are now non-trivial.

Comment 7

[Kris Maglione [:kmag]]

There's another drawback to this plugin: It only handles <script> tags with no type attribute, or type="text/javascript", neither of which we tend to use very much.

It might make sense to just create our own plugin, so we can also handle XUL and the other script types that we commonly use, and ideally warn when we see scripts with ;version= tags in their types. In the mean time, I'll send a PR to the plugin's author for better type attribute support.

I think this solution is better than nothing, for the time being, though.

Comment 8

[Gregory Szorc [:gps]]

Comment on attachment 8696279 [details]
MozReview Request: Bug 1230776: Support scripts from HTML files in eslint. r?gps

https://reviewboard.mozilla.org/r/27303/#review24765

::: python/mach_commands.py
(Diff revision 3)
> -ESLINT_PROMPT = '''
> -Would you like to use eslint
> -'''.strip()
> -
> -ESLINT_PLUGIN_MOZILLA_PROMPT = '''
> -eslint-plugin-mozilla is an eslint plugin containing rules that help enforce
> -JavaScript coding standards in the Mozilla project. Would you like to use this
> -plugin
> -'''.strip()
> -
> -ESLINT_PLUGIN_REACT_PROMPT = '''
> -eslint-plugin-react is an eslint plugin containing rules that help React
> -developers follow strict guidelines. Would you like to install it
> -'''.strip()
> -

If you are going to remove unused variables, please write a commit message saying they are unused so I don't have to spend time trying to figure out why you removed them.

Ideally, this should have been done as a separate commit, as the change is unrelated to the main objective of this patch.

::: python/mach_commands.py:156
(Diff revision 3)
> -    @CommandArgument('path', nargs='?', default='.',
> -        help='Path to files to lint, like "browser/components/loop" '
> +    @CommandArgument('-e', '--ext', default='[.js,.jsm,.jsx,.html]',
> +        help='Filename extensions to lint, default: "[.js,.jsm,.jsx,.html]".')
> -            'or "mobile/android". '
> -            'Defaults to the current directory if not given.')

I don't understand why you removed this.

Comment 9

[Kris Maglione [:kmag]]

https://reviewboard.mozilla.org/r/27303/#review24765

> If you are going to remove unused variables, please write a commit message saying they are unused so I don't have to spend time trying to figure out why you removed them.
> 
> Ideally, this should have been done as a separate commit, as the change is unrelated to the main objective of this patch.

It seemed related to me. I was adding a new plugin, and was about to add another one of these messages for it, when I checked and realized none of them were actually used.


I still should have mentioned it in the commit message, though. Sorry.

> I don't understand why you removed this.

Mainly because it was preventing me from testing this, since it took the argument to my `--plugin` flag and moved it to end of the arguments list, after my other paths, and it didn't actually serve a purpose anymore. It might have been better as a separate commit, but that commit would have evaporated after I rebased on top of bug 1230300 (which you'd just r+ed but hadn't landed yet), so I decided to just leave it as-is.

Comment 10

[Kris Maglione [:kmag]]

Comment on attachment 8696279 [details]
MozReview Request: Bug 1230776: Support scripts from HTML files in eslint. r?gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27303/diff/3-4/

Comment 11

[Gregory Szorc [:gps]]

Comment on attachment 8696279 [details]
MozReview Request: Bug 1230776: Support scripts from HTML files in eslint. r?gps

https://reviewboard.mozilla.org/r/27303/#review25033

You snuck in some test changes. They look sane enough to me.

Comment 12

[Kris Maglione [:kmag]]

(In reply to Gregory Szorc [:gps] from comment #11)
> Comment on attachment 8696279 [details]
> MozReview Request: Bug 1230776: Support scripts from HTML files in eslint.
> r?gps
> 
> https://reviewboard.mozilla.org/r/27303/#review25033
> 
> You snuck in some test changes. They look sane enough to me.

They were necessary for the tree to pass eslint. Those were the only HTML files I could fix with just trivial changes. I'll move them to a separate commit.

Thanks.

Comment 13

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/fx-team/rev/5803942d32dc6318004c4a9c17b51355aa1c0f57
Bug 1230776: Fix ESLint failures in HTML test files. r=trivial

https://hg.mozilla.org/integration/fx-team/rev/dd26de01e3d255d6f62e679a58a0113ac1128183
Bug 1230776: Support scripts from HTML files in eslint. r=gps

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5803942d32dc
https://hg.mozilla.org/mozilla-central/rev/dd26de01e3d2