Closed Bug 1217946 Opened 4 years ago Closed 4 years ago

Use react-dev when testing memory tools, and fix up model validations


(DevTools :: Memory, defect)

43 Branch
Not set


(firefox44 fixed)

Firefox 44
Tracking Status
firefox44 --- fixed


(Reporter: jsantell, Assigned: jsantell)




(1 file)

In bug 960776, react-dev will be used when DevToolsUtils.testing, or DEBUG_JS_MODULES is on, and we can set DTU.testing directly without having to wait for bug 1217503. The reason this is a separate patch is because we have some PropType assertion warnings when using react-dev right now.
Attachment #8680066 - Flags: review?(nfitzgerald)
Comment on attachment 8680066 [details] [diff] [review]

Review of attachment 8680066 [details] [diff] [review]:

FYI, the biggest bottleneck in the tree view's rendering after I added the DFS caching was emitting warnings. So good that we are fixing this, also might want to consider a way to use non-dev react again.

r=me with the comments below addressed

::: devtools/client/memory/components/list.js
@@ +21,5 @@
>    render() {
>      let { items, onClick, itemComponent: Item } = this.props;
>      return (
> +      dom.ul({ className: "list" },, index) => {

This one is just working around the warning, not fixing it. We have a changing number of items and can be added or removed between renders and it wants a key to help figure out which elements can be re-used. The sub-items should provide a key here.

::: devtools/client/memory/components/toolbar.js
@@ +40,3 @@
>            className: `select-breakdown`,
>            onChange: e => onBreakdownChange(,
> +        },{ name, displayName }) => dom.option({ value: name }, displayName))),

Again, these should have keys, not ...'d and inlined to trick react.

We even have a `name` that we can use as the key.
Attachment #8680066 - Flags: review?(nfitzgerald) → review+
Component: Developer Tools → Developer Tools: Memory
Backed out for various devtools test bustages:
Flags: needinfo?(jsantell)
Thanks Wes, fixed the issue from a bad rebase -- Nick's working on fixing our tests to prevent this from happening again to release this for next uplift
Flags: needinfo?(jsantell)
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.