Closed Bug 1325191 Opened 7 years ago Closed 7 years ago

Fix Eslint errors in devtools/server/*.js

Categories

(DevTools :: Shared Components, defect)

Product:
DevTools
Component:
Shared Components
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox53 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 53
Tracking Flags:
Tracking Status
firefox53 --- fixed

People

(Reporter: ntim, Assigned: ntim)

References

Details

Attachments

(2 files)

Bug 1325191 - Fix Eslint errors in devtools/server/*.js.
59 bytes, text/x-review-board-request
jryans
: review+
Details
Bug 1325191 - Fix ESLint errors in devtools/server/performance/.
59 bytes, text/x-review-board-request
jryans
: review+
Details 
      No description provided.
Comment on attachment 8820881 [details]
Bug 1325191 - Fix Eslint errors in devtools/server/*.js.

https://reviewboard.mozilla.org/r/100266/#review100892

Thanks for working on this clean up!

::: devtools/server/actors/utils/TabSources.js:304
(Diff revision 5)
> -    } else if (aSource.introductionType === "wasm") {
> +    } else if (source.introductionType === "wasm") {
>        // Wasm sources are not JavaScript. Give them their own content-type.
>        spec.contentType = "text/wasm";
> -    } else {
> +    } else if (url) {
> -      if (url) {
>          // There are a few special URLs that we know are JavaScript:

Fix comment indentation

::: devtools/server/actors/utils/TabSources.js:317
(Diff revision 5)
> -            let pathname = new URL(url).pathname;
> +          let pathname = new URL(url).pathname;
> -            let filename = pathname.slice(pathname.lastIndexOf("/") + 1);
> +          let filename = pathname.slice(pathname.lastIndexOf("/") + 1);
> -            let index = filename.lastIndexOf(".");
> +          let index = filename.lastIndexOf(".");
> -            let extension = index >= 0 ? filename.slice(index + 1) : "";
> +          let extension = index >= 0 ? filename.slice(index + 1) : "";
> -            if (extension === "xml") {
> +          if (extension === "xml") {
>                // XUL inline scripts may not correctly have the

Fix comment indentation

::: devtools/server/actors/utils/TabSources.js:325
(Diff revision 5)
> -            }
> +          } else if (extension === "js") {
> -            else if (extension === "js") {
> -              spec.contentType = "text/javascript";
> +            spec.contentType = "text/javascript";
> -            }
> +          }
> -          } catch (e) {
> +        } catch (e) {
>              // This only needs to be here because URL is not yet exposed to

Fix comment indentation

::: devtools/server/actors/utils/TabSources.js:336
(Diff revision 5)
> -            }
> +          }
> -          }
> +        }
> -        }
> +      }
> -      }
> +    } else {
> -      else {
>          // Assume the content is javascript if there's no URL

Fix comment indentation

::: devtools/server/actors/utils/TabSources.js:430
(Diff revision 5)
> -    return resolve(this._sourceMaps.get(aSource));
> +    return resolve(this._sourceMaps.get(source));
>    },
>  
>    /**
>     * Set a SourceMapConsumer for the source map for
> -   * |aSource|.
> +   * |source|.

Seems like this would fit on the previous line

::: devtools/server/actors/utils/map-uri-to-addon-id.js:28
(Diff revision 5)
>   * to worry about them.
>   */
>  if (!Services.appinfo
>      || Services.appinfo.processType == Services.appinfo.PROCESS_TYPE_CONTENT
> -    || Services.appinfo.ID === undefined /* XPCShell */
> +
> +    /* XPCShell */

Mmm, I don't really like this ESLint rule that bans inline comments...  To me, the previous version is more natural.  What do you think?

::: devtools/server/actors/utils/webconsole-listeners.js:1
(Diff revision 5)
> +"use strict";

Add MPL license header.

::: devtools/server/worker.js:51
(Diff revision 5)
>    switch (packet.type) {
>      case "connect":
>      // Step 3: Create a connection to the parent.
>        let connection = DebuggerServer.connectToParent(packet.id, this);
>        connections[packet.id] = {
> -        connection : connection,
> +        connection: connection,

Just `connection,` on this line should work
Attachment #8820881 - Flags: review?(jryans) → review+
Comment on attachment 8820881 [details]
Bug 1325191 - Fix Eslint errors in devtools/server/*.js.

https://reviewboard.mozilla.org/r/100266/#review100894

::: devtools/server/worker.js:48
(Diff revision 5)
>  
>  this.addEventListener("message", function (event) {
>    let packet = JSON.parse(event.data);
>    switch (packet.type) {
>      case "connect":
>      // Step 3: Create a connection to the parent.

Seems like the comment indentation could be improved here
Comment on attachment 8820896 [details]
Bug 1325191 - Fix ESLint errors in devtools/server/performance/.

https://reviewboard.mozilla.org/r/100280/#review100896

Looks like a good clean up, thanks for working on it!

::: devtools/server/performance/memory.js:196
(Diff revision 3)
>        return this._getCurrentTime();
>      }
>  
>      this._frameCache.initFrames();
>  
> -    this.dbg.memory.allocationSamplingProbability = options.probability != null
> +    this.drainAllocationsTimeoutTimer = typeof drainAllocationsTimeout === "number" ?

Maybe this `typeof` check isn't needed anymore?  Seems like you default to null already, and it would strange for someone to pass a non-number...

Seems like a language with types is a better fix than `typeof` for this. :)

::: devtools/server/performance/memory.js:196
(Diff revision 3)
>        return this._getCurrentTime();
>      }
>  
>      this._frameCache.initFrames();
>  
> -    this.dbg.memory.allocationSamplingProbability = options.probability != null
> +    this.drainAllocationsTimeoutTimer = typeof drainAllocationsTimeout === "number" ?

Hmm, it looks like the new version doesn't use the `probability` arg?  I would think it still needs to be set on `this.dbg.memory.allocationSamplingProbability`.

::: devtools/server/performance/profiler.js:242
(Diff revision 3)
>      isActive: function () {
>        let isActive = nsIProfilerModule.IsActive();
>        let elapsedTime = isActive ? nsIProfilerModule.getElapsedTime() : undefined;
>        let { position, totalSize, generation } = this.getBufferInfo();
> -      return { isActive: isActive, currentTime: elapsedTime, position, totalSize, generation };
> +      return {
> +        isActive: isActive,

Just `isActive,` should work

::: devtools/server/performance/profiler.js:566
(Diff revision 3)
>   * @param {function} handler
>   * @return {function}
>   */
>  function sanitizeHandler(handler, identifier) {
>    return DevToolsUtils.makeInfallible(function (subject, topic, data) {
> -    subject = (subject && !Cu.isXrayWrapper(subject) && subject.wrappedJSObject) || subject;
> +    subject = (subject && !Cu.isXrayWrapper(subject) && subject.wrappedJSObject)

Might be less mysterious as an if block instead?  Up to you.

Also, I would generally put the `||` on the previous line to clarify that the expression continues, but this case is really a bit unusual.
Attachment #8820896 - Flags: review?(jryans) → review+
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fe99e9ae7fce
Fix Eslint errors in devtools/server/*.js. r=jryans
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd069b26630c
Fix ESLint errors in devtools/server/performance/. r=jryans
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/88b62488ca30
Fix test_memory_allocations_* failures. r=bustage
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: