Closed
Bug 1217946
Opened 9 years ago
Closed 9 years ago
Use react-dev when testing memory tools, and fix up model validations
Categories
(DevTools :: Memory, defect)
Tracking
(firefox44 fixed)
RESOLVED
FIXED
Firefox 44
Tracking | Status | |
---|---|---|
firefox44 | --- | fixed |
People
(Reporter: jsantell, Assigned: jsantell)
References
Details
Attachments
(1 file)
13.71 KB,
patch
|
fitzgen
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8680066 -
Flags: review?(nfitzgerald)
Comment 2•9 years ago
|
||
Comment on attachment 8680066 [details] [diff] [review] 1217946-react-validation.patch 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" }, ...items.map((item, 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(e.target.value), > + }, ...breakdowns.map(({ 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+
Assignee | ||
Updated•9 years ago
|
Component: Developer Tools → Developer Tools: Memory
Backed out for various devtools test bustages: https://hg.mozilla.org/integration/fx-team/rev/0bf0e3c8f51e
Flags: needinfo?(jsantell)
Assignee | ||
Comment 6•9 years ago
|
||
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)
Comment 7•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/64767e3b5082
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox44:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•