Closed Bug 1270926 Opened 4 years ago Closed 4 years ago

Support ESLint on XHTML files

Categories

(Firefox Build System :: Lint and Formatting, enhancement)

enhancement
Not set

Tracking

(firefox51 fixed)

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: MattN, Assigned: MattN)

References

Details

Attachments

(3 files)

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

This would be very useful for mochitests and our in-content pages.
The first issue I see is that it doesn't like `<![CDATA[` in `<script>`
Assignee: nobody → MattN+bmo
Status: NEW → ASSIGNED
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.
Component: General → ESLint
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.
Flags: needinfo?(mratcliffe)
Attachment #8783162 - Flags: review?(dao+bmo) → review+
(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.
Flags: needinfo?(mratcliffe)
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.
Attachment #8783521 - Flags: review+
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 on attachment 8783161 [details]
Bug 1270926 - Support ESLint on XHTML files.

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

Lgtm.
Attachment #8783161 - Flags: review?(ahalberstadt) → review+
Pushed by mozilla@noorenberghe.ca:
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
Product: Testing → Firefox Build System
You need to log in before you can comment on or make changes to this bug.