1365414 - eslint-plugin-mozilla does not play nice if installed globally

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, defect)

Description

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

I like to have my eslint install be global so that I can use it across projects. As such I also install my plugins globally. eslint-plugin-mozilla does not work if installed globally.

This appears to be due to rootDir's getter[1] assuming the module will be located within a central repo. However, if the file being linted is within the repo, but the plugin is not the following happens: eslint walks upwards and eventually enables the moz plugin after finding the root config, the moz plugin then walks up and eventually hits the FS root, linting then fails.

I've adjusted eslint-plugin-mozilla locally to also check from the current working directory. I've also considered walking from the target file being linted. This enables more diverse usage of the plugin. Are there any reasons why not to do this? If no, I can prepare a patch.

[1]: https://dxr.mozilla.org/mozilla-central/source/tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js#525

Comment 1

[Mark Banner (:standard8)]

So the reason we need the root dir, is to try and find the "dynamic" files tools/lint/eslint/modules.json and browser/base/content/global-scripts.inc

I'm happy for a change to be made, as long as it still uses the "dynamic" files for m-c - the published version now falls back to local files, so if we hit the FS root, we could just declare it not found, and assume it isn't an m-c repo.

Comment 2

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Attachment: Bug 1365414 - Update eslint-plugin-mozilla to also search for MC root from CWD.

The eslint-plugin-mozilla currently searches for Mozilla Central root by
walking up from its installed dir. However, if the plugin is installed outside
of central, such as gloablly, it will not find the root. This changeset
retains that behaviour, but will also perform the same check from the current
working directly before failing.

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

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 8868727 [details]
Bug 1365414 - Update eslint-plugin-mozilla to also search for MC root from CWD.

https://reviewboard.mozilla.org/r/140328/#review143986

r=Standard8 with the changes I've suggested.

::: commit-message-d8762:5
(Diff revision 1)
> +Bug 1365414 - Update eslint-plugin-mozilla to also search for MC root from CWD. r?standard8
> +
> +The eslint-plugin-mozilla currently searches for Mozilla Central root by
> +walking up from its installed dir. However, if the plugin is installed outside
> +of central, such as gloablly, it will not find the root. This changeset

s/gloablly/globally/

::: tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js:545
(Diff revision 1)
> +
> +      let possibleRoot = searchUpForIgnore(path.dirname(module.filename));
> +      if (!possibleRoot) {
> +        possibleRoot = searchUpForIgnore(path.resolve());
> +      }
> +      if(!possibleRoot) {

nit: missing space (as picked up by eslint).

::: tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js:545
(Diff revision 1)
> +
> +      let possibleRoot = searchUpForIgnore(path.dirname(module.filename));
> +      if (!possibleRoot) {
> +        possibleRoot = searchUpForIgnore(path.resolve());
> +      }
> +      if(!possibleRoot) {

The if could potentially just be inside the other if, to avoid the double-check when it is found from the module location.

Comment 4

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Comment on attachment 8868727 [details]
Bug 1365414 - Update eslint-plugin-mozilla to also search for MC root from CWD.

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

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2be1833d89d2
Update eslint-plugin-mozilla to also search for MC root from CWD. r=standard8

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2be1833d89d2