Closed Bug 1228363 Opened 6 years ago Closed 6 years ago

"mach eslint devtools" is broken

Categories

(DevTools :: General, defect)

defect
Not set
normal

Tracking

(firefox45 fixed)

RESOLVED FIXED
Firefox 45
Tracking Status
firefox45 --- fixed

People

(Reporter: past, Assigned: miker)

Details

Attachments

(1 file, 1 obsolete file)

It appears to be related to the balanced-listeners rule from the mozilla plugin:

$ mach eslint devtools/
 0:00.11 Running /usr/bin/eslint in devtools/
 0:00.11 /usr/bin/eslint --ext [.js,.jsm,.jsx] .
/home/past/src/gecko/testing/eslint-plugin-mozilla/lib/rules/balanced-listeners.js:41
      type: node.arguments[0].value,
                             ^

TypeError: Cannot read property 'value' of undefined
    at addAddedListener (/home/past/src/gecko/testing/eslint-plugin-mozilla/lib/rules/balanced-listeners.js:41:30)
    at EventEmitter.CallExpression (/home/past/src/gecko/testing/eslint-plugin-mozilla/lib/rules/balanced-listeners.js:91:11)
    at emitOne (events.js:82:20)
    at EventEmitter.emit (events.js:169:7)
    at NodeEventGenerator.enterNode (/usr/lib/node_modules/eslint/lib/util/node-event-generator.js:42:22)
    at CommentEventGenerator.enterNode (/usr/lib/node_modules/eslint/lib/util/comment-event-generator.js:98:23)
    at Controller.controller.traverse.enter (/usr/lib/node_modules/eslint/lib/eslint.js:782:36)
    at Controller.__execute (/usr/lib/node_modules/eslint/node_modules/estraverse/estraverse.js:397:31)
    at Controller.traverse (/usr/lib/node_modules/eslint/node_modules/estraverse/estraverse.js:495:28)
    at EventEmitter.module.exports.api.verify (/usr/lib/node_modules/eslint/lib/eslint.js:779:24)

I see this on both Mac and Linux and is not fixed by "mach eslint --setup".
Thanks Panos.
This seems to happen with (at least) this file: devtools\client\performance\legacy\actors.js
On line 92, it does: `this._poller.on();`
And this tricks the rule into thinking it's a new listener being added.
So we should fix the rule to make sure it does not assume that all instances of '.on(' are actually event listeners.
Panos, do you mind reviewing this change?
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Attachment #8692582 - Flags: review?(past)
Bug 1228363 - stop mach eslint devtools throwing on empty on() and off() calls r?=past

The error is triggered by this._poller.on() because we assume we are adding an event listener and on() has no arguments.

Easy fix.
Attachment #8692584 - Flags: review?(past)
Assignee: pbrosset → mratcliffe
Attachment #8692582 - Attachment is obsolete: true
Attachment #8692582 - Flags: review?(past)
Comment on attachment 8692584 [details]
MozReview Request: Bug 1228363 - stop mach eslint devtools throwing on empty on() and off() calls r?=past

https://reviewboard.mozilla.org/r/26287/#review23731

Stealing review from :past.
Let's land this.
Attachment #8692584 - Flags: review+
Attachment #8692584 - Flags: review?(past)
https://hg.mozilla.org/mozilla-central/rev/e0a029b6cdde
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.