1412023 - Memory Tool needs to use ES6 classes so that we can switch it over to PureComponent

Status: RESOLVED FIXED

(DevTools :: Memory, enhancement, P2)

Comment 1

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

Attachment: Bug 1412023 - Memory Tool to ES6 classes

@gregtatum If you have made changes that would make it hard to land this then I can recreate the patch any time.

I did need to make a few manual changes to get this to work but it is probably a lot easier for me to regenerate the patch than for you to resolve any merge conflicts.

Because the changes are generic and mostly automated you don't need to take too long looking at every detail but you probably want to take a quick look over it. The main thing with a change like this is a green try.

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

Comment 2

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

Comment on attachment 8922760 [details]
Bug 1412023 - Memory Tool to ES6 classes

https://reviewboard.mozilla.org/r/193906/#review198982

DominatorTree.js and DominatorTreeItem.js were the only files where I needed to make manual changes. In both cases it just meant moving the class creation outside of createFactory e.g.

```
let factory = createFactory(createClass({
  ...
}));
```

Would have been changed to:

```
class FactoryClass extends Component {
  ...
}

let factory = createFactory(FactoryClass);
```

Comment 3

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

Comment on attachment 8922760 [details]
Bug 1412023 - Memory Tool to ES6 classes

https://reviewboard.mozilla.org/r/193906/#review198984

::: devtools/client/memory/components/DominatorTree.js:23
(Diff revision 1)
>  
>  /**
>   * A throbber that represents a subtree in the dominator tree that is actively
>   * being incrementally loaded and fetched from the `HeapAnalysesWorker`.
>   */
> -const DominatorTreeSubtreeFetching = createFactory(createClass({
> +class DominatorTreeSubtreeFetchingClass extends Component {

DominatorTree.js and DominatorTreeItem.js were the only files where I needed to make manual changes. In both cases it just meant moving the class creation outside of createFactory e.g.

```
let factory = createFactory(createClass({
  ...
}));
```

Would have been changed to:

```
class FactoryClass extends Component {
  ...
}

let factory = createFactory(FactoryClass);
```

Comment 4

[Greg Tatum [:gregtatum]]

Comment on attachment 8922760 [details]
Bug 1412023 - Memory Tool to ES6 classes

https://reviewboard.mozilla.org/r/193906/#review199020

Looks good, as long as you restore some of the mistakenly deleted comments.

::: devtools/client/memory/components/Heap.js:176
(Diff revision 1)
>    }
>  
>    return null;
>  }
>  
> -/**
> +class Heap extends Component {

This comment got deleted. Could you restore it?

::: devtools/client/memory/components/Individuals.js:13
(Diff revision 1)
>  const Tree = createFactory(require("devtools/client/shared/components/Tree"));
>  const DominatorTreeItem = createFactory(require("./DominatorTreeItem"));
>  const { TREE_ROW_HEIGHT } = require("../constants");
>  const models = require("../models");
>  
> -/**
> +class Individuals extends Component {

Comments here too.

::: devtools/client/memory/components/List.js:9
(Diff revision 1)
>  
>  "use strict";
>  
> -const { DOM: dom, createClass, PropTypes } = require("devtools/client/shared/vendor/react");
> +const { DOM: dom, Component, PropTypes } = require("devtools/client/shared/vendor/react");
>  
> -/**
> +class List extends Component {

Comments.

Comment 5

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

Comment on attachment 8922760 [details]
Bug 1412023 - Memory Tool to ES6 classes

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/193906/diff/1-2/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8dd4ccaed43e
Memory Tool to ES6 classes r=gregtatum

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/8dd4ccaed43e