1836439 - Tab react components render at almost any state change

Status: RESOLVED FIXED

(DevTools :: Debugger, enhancement)

Description

[Alexandre Poirot [:ochameau]]

The Tabs and Tab components currently re-render for almost all state changes because of lack of correct memoization of selectors, but also because <Tab> is being given props which always change (onDrag* event handler). Then they are all updated unecessarily because they have selectedLocation as prop, whereas it only impact deselected and newly selected tabs only.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1836439 - [devtools] Fix copy to clipboard tab context menu.

This was calling outdated selector name and wrong argument were passed to the action.

While I'm reading this code, also fix disabled state of two other context menus.
And also disable them based on Tab source rather than the selected source!

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1836439 - [devtools] Stop forcing Tab components to always rerender because of arrow function passed in props.

This isn't enough to stop all unecessary Tab updates, but may help cut a few.
Each time Tabs re-rendered, all Tab children component were forced to re-render
because all these onDrag* props were new objects.

Comment 3

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1836439 - [devtools] Move Tab context menu from React component to an action.

Comment 4

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1836439 - [devtools] Stop updating all Tab components on selected location change.

Computing the "isActive" field from the selector allows to update the component
only when the related source becomes selected or stops being selected.

Also, activeSearch being set to "source" no longer exists.

Last but not least, tweak Tab's key property to be unique per tab.
Using 'index' is wrong, when moving or adding tab first, it will change all Tab's state!

With all these patches, now, when you have more than two tabs,
when selecting another tab, only two Tab components gets re-rendered:
the old and the new being selected.

Comment 5

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1836439 - [devtools] Avoid unecessary state change when dragging tabs.

Sometimes the UI code emit an undefined new index.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/df036e39dd8c
[devtools] Fix copy to clipboard tab context menu. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/d36f1aac82d0
[devtools] Stop forcing Tab components to always rerender because of arrow function passed in props. r=devtools-reviewers,bomsy
https://hg.mozilla.org/integration/autoland/rev/514dc1087d5b
[devtools] Move Tab context menu from React component to an action. r=devtools-reviewers,bomsy
https://hg.mozilla.org/integration/autoland/rev/6fb412d1fc55
[devtools] Stop updating all Tab components on selected location change. r=devtools-reviewers,bomsy
https://hg.mozilla.org/integration/autoland/rev/bd2bde7ae3a9
[devtools] Avoid unecessary state change when dragging tabs. r=devtools-reviewers,bomsy

Comment 7

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/df036e39dd8c
https://hg.mozilla.org/mozilla-central/rev/d36f1aac82d0
https://hg.mozilla.org/mozilla-central/rev/514dc1087d5b
https://hg.mozilla.org/mozilla-central/rev/6fb412d1fc55
https://hg.mozilla.org/mozilla-central/rev/bd2bde7ae3a9