As part of rolling out ESLint across the tree, we should enable it for the widget/ directory.

I'm happy to mentor this bug. There's background on our eslint setups here:

https://developer.mozilla.org/docs/ESLint

Please note: We want to end up with two separate commits. One with the automatic changes, the second with the manual changes and the .eslintignore change. It helps with reviewing if these appear as a commit series in Phabricator.

Here's some approximate steps:

• In .eslintignore, remove the widget/headless/tests/** and widget/tests/** lines.
• Run eslint like this to fix most/all the issues:
  • ./mach eslint --fix widget
• Inspect the diff to make sure that the indentation of the lines surrounding the changes look ok
  • (especially where function arguments are spread across different lines)
• Create a commit of the work so far, e.g.
  • hg commit -m "Bug nnn - Enable ESLint for widget (automatic changes)" widget
• For the remaining issues, you'll need to fix them by hand. Most of those should be reasonably easy to understand, but there's more information on some specific bits (especially no-undef) here:
  • https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/ESLint#Common_issues_and_how_to_solve_them
  • http://eslint.org/docs/rules/
  • For the no-implied-val error, you can put this on the line before: // eslint-disable-next-line no-implied-val
• Create a second commit of the manual changes, e.g.
  • hg commit -m "Bug nnn - Enable ESLint for widget (manual changes)"
• Post the two commits via phabricator: https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html
  When you post, please request initial review from myself (standard8), I'll then redirect to the owners of the code if they look good.

If you need help, ask here or on the #introduction channel on IRC: https://wiki.mozilla.org/IRC

@standard8, I would like to work on it, I build the gecko on my laptop and working on it :)

Sure, please do.

Hi Mark, thanks for the assigning the issue , I fixed most of the issues , I have a doubt in

/widget/tests/test_imestate.html for no-shadow (eslint) error in this snippet. What should I rename that aTest in internal function?

https://pastebin.com/8MBMrtTA

Also for the errors like

1277:5 error Use Services.obs rather than getService(). mozilla/use-services (eslint)

We need to import the +ChromeUtils.import("resource://gre/modules/Services.jsm"); right?

Thanks

(In reply to Shivam Singhal [ :championshuttler ] from comment #3)

/widget/tests/test_imestate.html for no-shadow (eslint) error in this snippet. What should I rename that aTest in internal function?

I think I'd be tempted to change those aTestInner. This maintains the a-arg style, but also makes it a different name. TBH I'm not sure the test actually needs passing into those functions, but lets not change that at this stage.

Also for the errors like

1277:5 error Use Services.obs rather than getService(). mozilla/use-services (eslint)

We need to import the +ChromeUtils.import("resource://gre/modules/Services.jsm"); right?

Yes, that should work, although if you can use this form, that would be slightly better:

const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm", null);

Bug 1521928 - Enable ESLint for widget (manual changes)
Bug 1521928 - Enable ESLint for widget (automatic changes)

Depends on D18067

Depends on D18579

https://hg.mozilla.org/mozilla-central/rev/0e0638c8ef27
https://hg.mozilla.org/mozilla-central/rev/0176f4dc4345

@Standard8 looks like I need to attach one more patch due to my mistake.

https://phabricator.services.mozilla.com/rMCU0176f4dc4345f1e7620f3984a7be294757616b2b#inline-36

Here it should be addObserver it should be removeObserver.

Since we haven't had a merge happen in-between landing, I think we can just fix it here as you've already done. Generally though we'd probably just file a new bug.

https://hg.mozilla.org/mozilla-central/rev/e9d51144a284

Mark, are these tests something that it would make sense to uplift to beta?

No, this is just code style/formatting fixes, there's no actual bug fixes in here.