1326137 - update redux and react-redux to remove react-dev warnigs

Status: RESOLVED FIXED

(DevTools :: Shared Components, defect, P1)

Description

[[e-mail]]

After bug 1325914, the react-dev warnings from source code is resolved.

There are several `Stateless function components cannot be given refs` warnings as well, ex:

Warning: Stateless function components cannot be given refs (See ref "wrappedInstance" in ToggleButton created by Connect(ToggleButton)). Attempts to access this ref will fail.


StateOverflow said it could be fixed by upgrading to react-redux 4, which wont attach ref to the wrapped component by default

http://stackoverflow.com/questions/35952607/what-does-stateless-function-components-cannot-be-given-refs-mean

The redux / react-redux version are 11 months ago, we should update it to get benefit of .

Comment 1

[Marco Mucci [:MarcoM]]

Hi Fred, would like to confirm if this bug should be put in the reserve backlog?

Comment 2

[[e-mail]]

yes please put this into the reserve backlog, and its qe-verify-

Comment 3

[[e-mail]]

redux from 3.3 to 3.6 https://github.com/reactjs/redux/releases
react-redux from v4 or v3.0.1(the version is not specified in react_redux_update file, guessed by commit time) to 5.0.1 https://github.com/reactjs/react-redux

react-redux 5.0 claim a significant performance improvements on connect() so it's more attractive to update it.
https://github.com/reactjs/react-redux/pull/416

Comment 4

[Jarda Snajdr [:jsnajdr]]

Attachment: Bug 1326137 - Upgrade redux to version 3.6.0

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

Comment 5

[Jarda Snajdr [:jsnajdr]]

Attachment: Bug 1326137 - Upgrade react-redux to version 5.0.1

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

Comment 6

[Jarda Snajdr [:jsnajdr]]

Attached two patches that upgrade redux and react-redux to their latest versions, respectively. I didn't update the *_UPGRADING text files yet, I'm aware that it needs to be done before landing.

(In reply to Fred Lin [:gasolin] from comment #3)
> react-redux 5.0 claim a significant performance improvements on connect() so
> it's more attractive to update it.
> https://github.com/reactjs/react-redux/pull/416

Unfortunately, these performance improvements don't materialize in our case. I think that the reason is that these promised improvements are in cases when there is a lot (hundreds) of connect()-ed components and when they are in a parent-child relationship with each other. In that case, it matters a lot in what order the updates are done. And react-redux 5.0 optimizes that order.

In devtools, there is always just a handful of connect()-ed components, so the optimizations don't matter.

I ran two try runs with talos for the patch:

https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=5bad4416358cf3aa775c52e57891215e522c9639&newProject=try&newRevision=2b4603b71b935196d558fa75b343576dbf51be3f&framework=1&showOnlyImportant=0

https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=5bad4416358cf3aa775c52e57891215e522c9639&newProject=try&newRevision=41400ce6b4c2f6792c747fdb378f651939472cf3&framework=1&showOnlyImportant=0

None of them shows any significant changes performance-wise.

Comment 7

[Jarda Snajdr [:jsnajdr]]

Comment on attachment 8825323 [details]
Bug 1326137 - Upgrade redux to version 3.6.0

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

Comment 8

[Jarda Snajdr [:jsnajdr]]

Comment on attachment 8825324 [details]
Bug 1326137 - Upgrade react-redux to version 5.0.1

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

Comment 9

[Jarda Snajdr [:jsnajdr]]

Submitting final patches for review.

I sent an "intent to upgrade" announcement to the devtools mailing list on Jan 10 (yesterday). So far, I got one positive reply from jryans. My plan is to wait a few days and then land if no issues are raised.

Comment 10

[[e-mail]]

Comment on attachment 8825323 [details]
Bug 1326137 - Upgrade redux to version 3.6.0

https://reviewboard.mozilla.org/r/103500/#review104842

::: devtools/client/shared/vendor/REDUX_UPGRADING:12
(Diff revision 2)
> -REDUX_UPGRADING
> -
> -Current version of redux : 3.3.0
> -
> -1 - grab the unminified version of redux on npm. For release 3.3.0 for instance,
> -https://npmcdn.com/redux@3.3.0/dist/redux.js
> +How to upgrade:
> +1. git clone https://github.com/reactjs/redux - clone the repo
> +2. git checkout v3.6.0 - checkout the right version tag
> +3. npm install - compile the sources to a JS module file
> +4. cp dist/redux.js devtools/client/shared/vendor - copy the unminified JS file
> +5. update the current version in this file

very clear instruction +1

Comment 11

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8825324 [details]
Bug 1326137 - Upgrade react-redux to version 5.0.1

https://reviewboard.mozilla.org/r/103502/#review104916

LGTM!

R+ assuming Try is green

Honza

Comment 12

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8825323 [details]
Bug 1326137 - Upgrade redux to version 3.6.0

https://reviewboard.mozilla.org/r/103500/#review104918

R+ assuming Try is green.

Thanks Jarda for working on this!

Honza

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d2a185bd6efe
Upgrade redux to version 3.6.0 r=Honza
https://hg.mozilla.org/integration/autoland/rev/f649856d2281
Upgrade react-redux to version 5.0.1 r=Honza

Comment 14

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/d2a185bd6efe
https://hg.mozilla.org/mozilla-central/rev/f649856d2281

Comment 15

[[e-mail]]

The warnings are gone, thanks!