1239438 - fix eslint errors in styles.js

Status: RESOLVED FIXED

(DevTools :: Inspector: Rules, defect)

Description

[Tom Tromey :tromey]

I happened to look at styles.js today and was puzzled by an eslint
error.  This turned into bug 1239426.

Once I fixed that I went ahead and fixed all the easy eslint reports
in styles.js while I was there.

After the patch I'll attach, there are still two reports:

  1018:5  error  "PageStyleFront" is defined but never used  no-unused-vars
  1657:5  error  "StyleRuleFront" is defined but never used  no-unused-vars

Perhaps these should just not be given names.

Comment 1

[Tom Tromey :tromey]

Attachment: fix most eslint warnings in styles.js

Comment 2

[Tom Tromey :tromey]

Comment on attachment 8707597 [details] [diff] [review]
fix most eslint warnings in styles.js

Mostly just basic stuff.

I wasn't sure the best way to deal with the unused function arguments,
so I chose to document the parameters as they are passed -- this code
defining the interface used by the rule rewriter.

Also, the front code, like this:

var StyleRuleFront = protocol.FrontClass(StyleRuleActor, {

... causes a warning, because StyleRuleFront is never referenced.

One idea would be to remove the declaration, so:

protocol.FrontClass(StyleRuleActor, {

I felt this might be less clear; but on the other hand I suppose I could
mention StyleRuleFront in a comment and thus remove the last two warnings.
What do you think?

Comment 3

[Patrick Brosset <:pbro>]

(In reply to Tom Tromey :tromey from comment #2)
> Also, the front code, like this:
> 
> var StyleRuleFront = protocol.FrontClass(StyleRuleActor, {
> 
> ... causes a warning, because StyleRuleFront is never referenced.
> 
> One idea would be to remove the declaration, so:
> 
> protocol.FrontClass(StyleRuleActor, {
> 
> I felt this might be less clear; but on the other hand I suppose I could
> mention StyleRuleFront in a comment and thus remove the last two warnings.
> What do you think?
Some fronts use exports.MyFront = MyFront, but that's because they are top-level fronts that need to be instantiated by the UI (e.g. ReflowFront in the layout-view).
But in this particular case, the front doesn't need to be instantiated since one of the parent actors creates StyleRuleActor instances and sends this as part of response packets.

If you think it's clearer to keep this var declaration anyway, I see 2 options:
use exports.StyleRuleFront = StyleRuleFront even if no one uses this exported symbol (after all, we might use it from tests at some stage).
or use: /* exported StyleRuleFront */ which tells eslint not to warn about this. But that means we have to manually maintain this comment.

Personally, I'm ok to remove the var declaration.

Comment 4

[Tom Tromey :tromey]

Attachment: fix most eslint warnings in styles.js

Updated to remove the declarations.

Comment 5

[Tom Tromey :tromey]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0e53ed1fc58e

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/546b7fdcb156

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/546b7fdcb156

Comment 8

[Mayur Patil]

[bugday-20160323]

Status: RESOLVED,FIXED -> UNVERIFIED

Comments:
STR: Not clear.
Developer specific testing

Component: 
Name			Firefox
Version			46.0b9
Build ID		20160322075646
Update Channel          beta
User Agent		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS			Windows 7 SP1 x86_64

Expected Results: 
Developer specific testing

Actual Results: 
As expected