Use non-browser environment for jsms

RESOLVED FIXED in Firefox 59

Status

enhancement
P2
normal
RESOLVED FIXED
2 years ago
Last year

People

(Reporter: Gijs, Assigned: standard8)

Tracking

(Blocks 1 bug)

Trunk
mozilla59
Dependency tree / graph

Firefox Tracking Flags

(firefox59 fixed)

Details

Attachments

(1 attachment)

`window` and `document` and friends are not available in jsms, and eslint should warn about their use just like any other undefined globals.
Also xref bug 1293298 comment 5 where we get other globals wrong in various instances.
We should totally do it. I think the config options in eslint 4 would make this easier. Setting low priority for now until we get the upgrade.
Depends on: 1371293
Priority: -- → P4
Bumping priority as ESLint 4 should land next week.
Priority: P4 → P2
The bugs I've just filed as blocking this are ones that I've found from an initial brief experimentation with ESLint 4 and the glob configuration.

Most of the rest of the failures are in relation to globals available in jsms that ESLint doesn't know about (either exposed by default/webidl or imported via Cu.importGlobalProperties) - we'll need to expand our capabilities to cope with those.
We might need the whitelist from bug 1415483, to be able to handle globals correctly, unless the existing undefineds are all already covered by Cu.importGlobalProperties.
Depends on: 1415483
I'm starting to look into this, though it might take a week or so to get something fully together.
Assignee: nobody → standard8
Commit pushed to master at https://github.com/mozilla/activity-stream

https://github.com/mozilla/activity-stream/commit/5b02c1f5799a43dfbb4710612d90b4433bda0107
Merge pull request #3908 from Standard8/undef

Prepare for Bug 1369722 - Use non-browser environment for jsms
Depends on: 1426203
(In reply to Mark Banner (:standard8) from comment #4)
> Most of the rest of the failures are in relation to globals available in
> jsms that ESLint doesn't know about (either exposed by default/webidl or
> imported via Cu.importGlobalProperties) - we'll need to expand our
> capabilities to cope with those.

I've been thinking about this over the last few weeks, and working on a patch. So far I've got Cu.importGlobalProperties being detected and generating globals.

I was thinking of somehow parsing the webidl globals, but I haven't had time to look at that yet. I'm also starting to think it may be better for the build system to produce a list that we can then import (since a lot seems to be generated at build time already).

For now, I've manually generated a list based on the webidl files and created a new mozilla/jsm environment. This gets the immediate improvement of better globals detection for jsm files, and we can work out how to automate it later (if possible).
Comment on attachment 8937775 [details]
Bug 1369722 - Disable ESLint browser environment for jsm files.

https://reviewboard.mozilla.org/r/208486/#review214322

Awesome!
Attachment #8937775 - Flags: review?(dtownsend) → review+
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3052b1df15ef
Disable ESLint browser environment for jsm files. r=mossop
@Mardak: Since this is already landed in your repo, and the patch here included that fix, I went ahead and landed this. Hopefully that doesn't affect your exports too much.
https://hg.mozilla.org/mozilla-central/rev/3052b1df15ef
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
(In reply to Mark Banner (:standard8) (mainly afk until 3rd Jan) from comment #12)
> @Mardak: Since this is already landed in your repo, and the patch here
> included that fix, I went ahead and landed this. Hopefully that doesn't
> affect your exports too much.
Oh, turns out I wasn't CCed. Bug 1426203 export landed without issue though as the change here was already in m-c by the time we made the patch for export, so there was no diff to browser/extensions/activity-stream/common/PerfService.jsm to conflict. (And because your PR already merged to activity-stream, the change wasn't accidentally reverted during export.)
Product: Testing → Firefox Build System
You need to log in before you can comment on or make changes to this bug.