Closed Bug 1434982 Opened 6 years ago Closed 6 years ago

no-undef errors on ChromeUtils.import of module into global scope

Categories

(Developer Infrastructure :: Lint and Formatting, defect)

3 Branch
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: ato, Unassigned)

References

Details

Following the integration of
https://bugzilla.mozilla.org/show_bug.cgi?id=1431533 in central,
I get 443 no-undef errors when linting testing/marionette:

> % ./mach lint -funix testing/marionette/ | wc -l
> 443

Examples:

> % ./mach lint -funix testing/marionette
> testing/marionette/accessibility.js:13:16: no-undef error: 'Log' is not defined.
> testing/marionette/accessibility.js:23:1: no-undef error: 'XPCOMUtils' is not defined.
> testing/marionette/accessibility.js:176:11: no-undef error: 'Services' is not defined.
> testing/marionette/accessibility.js:185:7: no-undef error: 'Services' is not defined.
> testing/marionette/action.js:362:5: no-undef error: 'assert' is not defined.

They arise from code such as this:

> ChromeUtils.import("resource://gre/modules/Log.jsm");
> const logger = Log.repository.getLogger("Marionette");
It’s unclear to me how the ES job on TC passes when "./mach lint"
locally does not?
Depends on: 1431533
Flags: needinfo?(standard8)
I see the same problem in browser/base/content and other modules.
Perhaps https://bugzilla.mozilla.org/show_bug.cgi?id=1431533 fixed
the ES job running on TC, but it appears "./mach lint" picks up
its rules in a slightly different way?
Flags: needinfo?(kmaglione+bmo)
This is working fine for me locally.

I'm wondering if you repository hasn't updated tools/lint/eslint/eslint-plugin-mozilla somehow, or maybe you've got a hard copy of it in node_modules?

Maybe try nuking topsrcdir/node_modules and running `./mach eslint --setup` then lint again.
Flags: needinfo?(standard8)
As Mark said, it sounds like `mach lint` is using an outdated version of the plugin. It might be worth filing a bug if that command doesn't handle updates automatically, the way that `mach eslint` does.
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: needinfo?(kmaglione+bmo)
Resolution: --- → WORKSFORME
I should clarify a bit what is likely going on here:

- package.json lists file references to eslint-plugin-mozilla and eslint-plugin-spidermonkey-js, i.e. the in-tree versions.
- Doing an `npm install` (which is basically what `./mach eslint --setup` does) should install into node_modules all the normal modules, but symlink the file references (eslint-plugin-mozilla & eslint-plugin-spidermonkey-js)
- Then, when you update the tree, the in-tree versions automatically update & due to the symlinks, the command gets the right version.

Note that `./mach lint` basically calls through to `./mach eslint` for eslint - they're pretty much the same.

I believe all supported versions of npm (from around v3 onwards iirc) support symlinking, obviously some older Window disk formats don't, but I think most newer ones should be fine.
(In reply to Mark Banner (:standard8) from comment #3)

> I'm wondering if you repository hasn't updated
> tools/lint/eslint/eslint-plugin-mozilla somehow, or maybe you've
> got a hard copy of it in node_modules?
> 
> Maybe try nuking topsrcdir/node_modules and running `./mach eslint
> --setup` then lint again.

Nuking $(topsrcdir)/node_modules did the trick.

The npm version I have installed (on Linux, which supports
symlinking) is 3.10.10.  Obviously I didn’t check whether
eslint-plugin-mozilla was symlinked before I nuked node_modules,
but if there are systems that don’t support this (pre 3.x npm and
Windows), shouldn’t the build system have to take care of updating
the files automatically?
Flags: needinfo?(standard8)
We currently have a min version for node of 6.9.1, which shipped with npm 3.10.8 by default. Getting an earlier version would likely be difficult, so I'm not too worried about that case (I think npm 2 might have supported symlinks as well, but can't remember atm).

For Windows, we used to have the support but removed it, partially due to the added complexity. I haven't heard any complaints, so I'm generally assuming most of our developers are on modernish systems.
Flags: needinfo?(standard8)
Product: Testing → Firefox Build System
Version: Version 3 → 3 Branch
Product: Firefox Build System → Developer Infrastructure
You need to log in before you can comment on or make changes to this bug.