1253650 - Fix React propType warnings

Status: RESOLVED FIXED

(DevTools :: Memory, defect, P2)

Description

[Lin Clark [:linclark]]

We want to fail tests on propType warnings (Bug 1249106). It looks like some were recently introduced in the memory tool, so we need to clean those up first.

Comment 1

[Lin Clark [:linclark]]

Attachment: Bug1253650.patch

This makes the propTypes check pass.

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e5ee4919bfb6

Comment 2

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8727018 [details] [diff] [review]
Bug1253650.patch

Review of attachment 8727018 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks! r=me with comments below

::: devtools/client/memory/components/toolbar.js
@@ +11,5 @@
>  
>  module.exports = createClass({
>    displayName: "Toolbar",
>    propTypes: {
>      censusDisplays: PropTypes.arrayOf(PropTypes.shape({

Can you change this to

  PropTypes.arrayOf(censusDisplayModel)

@@ +26,5 @@
>      diffing: models.diffingModel,
>      onToggleDiffing: PropTypes.func.isRequired,
>      view: PropTypes.string.isRequired,
>      onViewChange: PropTypes.func.isRequired,
>      dominatorTreeDisplays: PropTypes.arrayOf(PropTypes.shape({

And this to:

  PropTypes.arrayOf(dominatorTreeDisplayModel)

Comment 3

[Lin Clark [:linclark]]

Those variables aren't defined in toolbar.js. I don't think they are exported from models.js in a way that makes sense to reuse them, currently.

Would it be ok to land this as is and create a follow up for that change? That would make it possible to get Bug 1249106 landed.

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Sure

Comment 5

[Lin Clark [:linclark]]

Great, thanks. Created Bug 1254242 as the follow up.

Comment 6

[Pulsebot]

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

Comment 7

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

https://hg.mozilla.org/mozilla-central/rev/50d6d1b53a1d