Closed satazor closed 7 years ago
Hi ! you're right ;-)
But we probably should use memoization on 'getP' using 'createSelector' and composing with 'getLocale' and 'getPhrases'. so object returned will be the same until locale or phrases changes.
thanks for reporting :)
2017-01-22 1:28 GMT+01:00 André Cruz notifications@github.com:
Hello!
I've been using the translate() enhancer for quite some time and, after some experiments, I concluded that all components using translate are being re-rendered after any dispatch(action).
All my components extend PureComponent which goes well with redux because it avoids re-rendering components unless props have changed. The issue here is that the p object changes every time because of this line: https://github.com/Tiqa/redux-polyglot/blob/master/src/selectors.js#L52. A new object is being created in mapStateToProps function which causes p ref to change which makes all translated components to re-render.
The obvious solution here is to implement a custom shouldComponentUpdate function that does a shallow compare for all properties, except for p which needs to be deeply compared. While it works, it feels awkward to delegate that responsibility to devs.
Am I missing something?
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Tiqa/redux-polyglot/issues/71, or mute the thread https://github.com/notifications/unsubscribe-auth/AQHWeqaFnnuJ8NUKy1MjpUcQMEqz-ec7ks5rUqKpgaJpZM4LqOoF .
--
Guillaume ARM +33 6 71 50 10 11 www.redpelicans.com
@guillaumearm sounds good! I'm getting huge perf issues on inputs, where a dispatch is being issued on every keypress. Do you have some free time to solve this issue? I can step in but I guess it would take much more time to do it.
ok i will publish a new release candidate tonight about this ;-)
I just published a patch on next
npm dist-tag.
try it with a npm install redux-polyglot@next
and tell me if it's better ;-)
@guillaumearm I've tested the next
dist tag and I see no difference. :(
hum too bad. I will write more tests about react performance soon, and find a way to solve that. stay tuned ;-)
@satazor, here: https://github.com/Tiqa/redux-polyglot/blob/optimize/getP/src/selectors.js#L69 I think it's because second parameter getP object is recreated all the time.
I will try to use a memoized merge object to fix this. what do you think ?
another solution would be to use createSelectorCreator
from reselect
to create a custom createSelector.
done here : https://github.com/Tiqa/redux-polyglot/pull/74/commits/d69a9afc5a3cdb95c6eaeba42bddc06d5ecefa51
I just published the 0.4.2-1
version on next
npm dist-tag ;-)
Try it, and tell me if it's better ;-) Can you provide me your tests ? if you got some.
I'm testing on a proprietary project but it's quite simple to test, just export a translated pure component and do a console.log inside render. You will see the print everytime you dispatch something (non related to polyglot).
Thanks for this, will test in a few hours.
@guillaumearm sorry for the delay. The behavior is still the same, will try to provide a fix.
@guillaumearm I've added a test case in #75, will investigate a fix. Want to do this together? :D
Yup, sure ^^ But not tonight, I'm busy anyway, thank you very much for your help :)
found it ! just need to memoize all getTranslation* functions. I will publish a new release soon ^^
Merged, it is seems to be OK.
@satazor you can test it on next
npm dist-tag ;-)
I'm currently on my phone, but will test asap.
I confirm it's working! Thanks for fixing this!
Hello!
I've been using the
translate()
enhancer for quite some time and, after some experiments, I concluded that all components usingtranslate
are being re-rendered after anydispatch(action)
.All my components extend
PureComponent
which goes well with redux because it avoids re-rendering components unless props have changed. The issue here is that thep
object changes every time because of this line: https://github.com/Tiqa/redux-polyglot/blob/master/src/selectors.js#L52. A new object is being created inmapStateToProps
function which causesp
ref to change and, consequently, causes all translated components to re-render.The user-land solution here is to implement a custom
shouldComponentUpdate
function that does a shallow compare for all properties except forp
, which needs to be deeply compared. While it works, it feels awkward to delegate that responsibility to devs.Am I missing something?