1250205 - Fix eslint warnings in devtools/client/inspector/markup/markup.js

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P1)

Description

[Patrick Brosset <:pbro>]

667:15  warning  "prev" is already declared in the upper scope       no-shadow
   680:15  warning  "next" is already declared in the upper scope       no-shadow
   958:25  warning  "added" is already declared in the upper scope      no-shadow
  1504:1   warning  Line 1504 exceeds the maximum line length of 80     max-len
  1519:13  warning  "container" is already declared in the upper scope  no-shadow
  2870:14  warning  "val" is already declared in the upper scope        no-shadow
  2910:17  warning  "name" is already declared in the upper scope       no-shadow

These seem to confuse people when looking at logs.
I think we should, anyway, change all warnings to errors and get rid of them all (see bug 1246904).
But in the meantime, let's use this bug to get rid of the warnings.

Comment 1

[Patrick Brosset <:pbro>]

So, ESLint no-shadow rule complains about this:

switch (something) {
  case "foo":
    let a = 1;
    break;
  case "bar":
    let a = 2;
    break;
}

The second 'a' variable is reported as already declared in the upper scope. This is a bit weird, but I guess I can simply declare a single variable outside of the switch.

Comment 2

[Patrick Brosset <:pbro>]

Attachment: MozReview Request: Bug 1250205 - Clean remaining eslint warnings in devtools/client/inspector/markup/markup.js; r=miker

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

Comment 3

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 8722092 [details]
MozReview Request: Bug 1250205 - Clean remaining eslint warnings in devtools/client/inspector/markup/markup.js; r=miker

https://reviewboard.mozilla.org/r/35861/#review32591

(In reply to Patrick Brosset [:pbrosset] [:pbro] from comment #1)
> So, ESLint no-shadow rule complains about this:
> 
> switch (something) {
>   case "foo":
>     let a = 1;
>     break;
>   case "bar":
>     let a = 2;
>     break;
> }
> 
> The second 'a' variable is reported as already declared in the upper scope.
> This is a bit weird, but I guess I can simply declare a single variable
> outside of the switch.

Yup, the scope in a switch is the space between one curly brace and another but people tend to think that each case has it's own scope.

Anyhow, an easy r+

Comment 4

[Patrick Brosset <:pbro>]

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

Comment 5

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

https://hg.mozilla.org/mozilla-central/rev/cad377e8dffc