redux-connect icon indicating copy to clipboard operation
redux-connect copied to clipboard

Changes to context do not propagate

Open arasmussen opened this issue 7 years ago • 14 comments

I believe this is a problem with asyncConnect containers where updating context does not properly propagate updates to children. I had to create a wrapper around react-redux's connect to get context changes to propagate. Do I need to do something similar for asyncConnect?

arasmussen avatar Feb 08 '17 00:02 arasmussen

It looks like this library directly uses react-redux's connect, how would I go about injecting my own connect function instead? Or somehow passing the {pure: false} setting so it knows to update child components on context changes...

arasmussen avatar Feb 08 '17 00:02 arasmussen

Ended up making a similar wrapper for the asyncConnect decorator and context updates propagate once again:

import { asyncConnect } from 'redux-connect';

export default function myAsyncConnect(asyncItems, mapStateToProps, mapDispatchToProps) {
  return asyncConnect(asyncItems, mapStateToProps, mapDispatchToProps, null, {pure: false});
}

arasmussen avatar Feb 08 '17 00:02 arasmussen

Feel free to close, might be cool to add this to FAQ or somewhere indexable by Google for "context updates don't propagate asyncConnect / redux". Thanks for the awesome lib!

arasmussen avatar Feb 08 '17 00:02 arasmussen

There is no need to wrap like this, asyncConnect accepts exactly the same args as connect, and its actually documented ;)

AVVS avatar Feb 08 '17 00:02 AVVS

https://github.com/makeomatic/redux-connect/blob/master/docs/API.MD#asyncconnect-decorator

AVVS avatar Feb 08 '17 00:02 AVVS

Sure, but it's cleaner to just do

asyncConnect([
  Container.asyncConnect,
])

rather than

asyncConnect([
  Container.asyncConnect,
], null, null, null, {pure: false})

all over the codebase. Containers shouldn't have to remember to pass {pure: false} just to get context updates to propagate... Seems like a bug in react-redux's connect tbh.

arasmussen avatar Feb 08 '17 00:02 arasmussen

Thats not a bug, its a performance feature. Pure means it does shallow props check and if no props changed then it wouldnt rerender. Shallow means it doesnt check nested properties of the object, but merely checks their references. Because context itself doesnt change, but instead it has a changed prop inside it - your component doesnt see it, hence it doesnt update without the pure: false setting. So if you set it to false you are really killing a lot of CPU of your users

AVVS avatar Feb 08 '17 00:02 AVVS

I'm not sure what you mean by context itself not changing though. My context changes from {obj: {foo: 'bar'}} to {obj: {foo: 'baz'}} and updates don't propagate to components who care about obj.foo and specify that in their contextTypes. How's that not a bug?

arasmussen avatar Feb 08 '17 00:02 arasmussen

Read more here - https://github.com/reactjs/react-redux/blob/master/docs/api.md#connectmapstatetoprops-mapdispatchtoprops-mergeprops-options

Its a basic performance feature. Your context is not checked beyond the upmost references. It might be counter intuitive, but you have to understand that every time state changes - every connected component render will be called, and your state changes upon every action there is. So, effectively, when you set something to pure: false - this component will rerender every time your state changes. Literally on any action

AVVS avatar Feb 08 '17 00:02 AVVS

I am changing context via Object.assign({}, obj, {foo: 'baz'}) so it should be completely changing the reference. Maybe React is being "smart" and changing keys on the underlying object under the hood? I'll take a look at the link.

So, effectively, when you set something to pure: false - this component will rerender every time your state changes. Literally on any action

You mean if a sub-component changes state then the whole tree will re-render? What else can I do to get context updates to propagate without that perf hit?

arasmussen avatar Feb 08 '17 00:02 arasmussen

Not much, really. You are fine if you don’t have that many non-pure components. But if you rely on your context too heavily - you should pass that data as props instead

On Feb 7, 2017, at 4:45 PM, Andrew Rasmussen [email protected] wrote:

I am changing context via Object.assign({}, obj, {foo: 'baz'}) so it should be completely changing the reference. Maybe React is being "smart" and changing keys on the underlying object under the hood? I'll take a look at the link.

So, effectively, when you set something to pure: false - this component will rerender every time your state changes. Literally on any action

You mean if a sub-component changes state then the whole tree will re-render? What else can I do to get context updates to propagate without that perf hit?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/makeomatic/redux-connect/issues/106#issuecomment-278193188, or mute the thread https://github.com/notifications/unsubscribe-auth/ABol0bmk4PMeeObOYkJcJib5uJUylx4Aks5raRAfgaJpZM4L6NRn.

AVVS avatar Feb 08 '17 01:02 AVVS

Hmm. I use context for the current viewer and company (customer), pretty much everything else is passed via props. That being said, every single container I use that fetches data (14 of them) is set to {pure: false} to fix this context propagation issue throughout my app. I don't perceive any performance issues on desktop or mobile. Do you think this is a major issue? I don't really see another great workaround besides passing viewer/company down as props...

arasmussen avatar Feb 08 '17 02:02 arasmussen

if they dont change often, that still should be fine, JS is obviously fast enough and would do hundreds of thousands of evaluations per second (modern CPUs 💃 ), so you wouldnt really notice it - all the underlaying components should still be pure and wont rerender

AVVS avatar Feb 08 '17 02:02 AVVS

Yeah they pretty much only update when the user explicitly edits one of them and they get reloaded. Which is only on a few specific components (edit user, edit company, login, logout, etc). Cool man, thanks for your responsiveness and helpfulness!

arasmussen avatar Feb 08 '17 02:02 arasmussen