1270926 - Support ESLint on XHTML files

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, enhancement)

Description

[Matthew N. [:MattN]]

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

This would be very useful for mochitests and our in-content pages.

Comment 1

[Matthew N. [:MattN]]

The first issue I see is that it doesn't like `<![CDATA[` in `<script>`

Comment 2

[Matthew N. [:MattN]]

I filed an issue with the HTML plugin to point out that they don't really handle XHTML properly (like XML): https://github.com/BenoitZugmeyer/eslint-plugin-html/issues/20

If we don't want to wait for an upstream fix we can probably do one of the following:
* Manually insert JS comment double-slashes before the opening and closing of CDATA sections and remove ones which aren't necessary.
* Make a plugin to automatically insert `//` before the opening and closing of the CDATA section. Not sure if this is possible yet.

Comment 3

[Matthew N. [:MattN]]

Attachment: Bug 1270926 - Support ESLint on XHTML files.

Review commit: https://reviewboard.mozilla.org/r/73098/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/73098/

Comment 4

[Matthew N. [:MattN]]

Attachment: Bug 1270926 - Modify aboutNetError.xhtml & blockedSite.xhtml to pass current eslint rules.

eol-last requires a new line at the end of the JS block

Review commit: https://reviewboard.mozilla.org/r/73100/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/73100/

Comment 5

[Matthew N. [:MattN]]

IIUC, I'm supposed to run tools/lint/eslint/update but I'm not super familiar with shrinkwrap and I don't know where documentation on upgrading a plugin is.

I also don't seem to be able to get a token for tooltool.upload.public with my LDAP, only tooltool.download.public.

Comment 6

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

(In reply to Matthew N. [:MattN] from comment #5)
> IIUC, I'm supposed to run tools/lint/eslint/update but I'm not super
> familiar with shrinkwrap and I don't know where documentation on upgrading a
> plugin is.
> 
> I also don't seem to be able to get a token for tooltool.upload.public with
> my LDAP, only tooltool.download.public.

I believe that the automation guys are dealing with that these days.

Comment 7

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: shrinkwrapped.diff

I have updated the shrinkwrap for your patches and uploaded the eslint archive to tooltool.

Just push this patch along with yours and it will be fine.

Comment 8

[Andrew Halberstadt [:ahal]]

Yeah, you need to request special scopes to upload to tooltool. You can file a bug to get them if you really want, otherwise feel free to ping me (sorry was PTO end of last week).

In bug 1288225 :jryans made it so the task used the in-tree eslint-plugin-mozilla. We considered vendoring in all of the 3rd party dependencies as well, but ultimately punted because it would have been moderately complicated to do.

Comment 9

[Andrew Halberstadt [:ahal]]

Comment on attachment 8783161 [details]
Bug 1270926 - Support ESLint on XHTML files.

https://reviewboard.mozilla.org/r/73098/#review71026

Lgtm.

Comment 10

[Matthew N. [:MattN]]

Thank everyone. Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ef42965c62af77266da13e6144e62b63ae10ee10

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/e54f7b23e452
Support ESLint on XHTML files. r=ahal
https://hg.mozilla.org/integration/fx-team/rev/8716cd004f68
Modify aboutNetError.xhtml & blockedSite.xhtml to pass current eslint rules. r=dao
https://hg.mozilla.org/integration/fx-team/rev/7703bf818488
Shrinkwrap eslint support on XHTML files r=mratcliffe

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e54f7b23e452
https://hg.mozilla.org/mozilla-central/rev/8716cd004f68
https://hg.mozilla.org/mozilla-central/rev/7703bf818488