Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Since upgrading to v6.0.0, components connected via connectAdvanced are rerendering with unrelated action dispatch #1118

Closed
dereknelson opened this issue Dec 12, 2018 · 4 comments

Comments

@dereknelson
Copy link

commented Dec 12, 2018

To preface, between ~80 different screens and ~15 different reducers, I love redux. Major props for all the hard work on v6.

What is the current behavior?
Using React's profiling tool and a snippet I found online:

const userTiming = (store) => (next) => (action) => {
    if (performance.mark === undefined) return next(action)
    performance.mark(`${action.type}_start`);
    const result = next(action);
    performance.mark(`${action.type}_end`);
    performance.measure(`${action.type}`,`${action.type}_start`,`${action.type}_end`,)
    return result
}

I noticed that a connected screen and it's list items (list items are also connected components) are rerendering when the action is modifying a completely unrelated reducer. The screen is connected via connectAdvanced, and uses forwardRef, but is rerendering even without the forwardRef. The screen does not rerender when it is connected in a normal fashion

What is the expected behavior?
The connected screen and its children not to rerender when unrelated actions are dispatched.

Which versions of Redux, and which browser and OS are affected by this issue? Did this work in previous versions of Redux?
This is in react-native

"react-redux": "^6.0.0",
"redux": "^4.0.1",
@markerikson

This comment has been minimized.

Copy link
Contributor

commented Dec 12, 2018

Please provide a project that reproduces the issue, preferably as a CodeSandbox or RN Snack. Not much we can do otherwise.

Couple other items. Can you clarify what specific behavior you're seeing? Can you confirm that your own components are re-rendering? Also, what's your use case for using connectAdvanced instead of the standard connect ?

@dereknelson

This comment has been minimized.

Copy link
Author

commented Dec 12, 2018

@markerikson will work on a repro, just wanted to get it out there because I noticed in the v6 changelog there was a part that said

Ensure connectAdvanced only re-renders if derived props change

so I didn't know if it was simple fix that someone missed.

Specifically, my components are rerendering when an action modifies a reducer that my screen is in no way connected to.

Can you confirm that your own components are re-rendering?

If you mean the instance of my component and not just the HOC that connects my component to the store, then yes. Going back to the FETCH_NOTIFICATIONS_SUCCESS action I console logged when the action is returned and the render method of my connected screen

Also, what's your use case for using connectAdvanced instead of the standard connect ?

Entirely so that I can use forwardRef

@dereknelson

This comment has been minimized.

Copy link
Author

commented Dec 12, 2018

As it turns out, when using connectAdvanced, the developer assumes all responsibility for memoizing the props, which was why my component with rerendering - all the props it was passed was a new reference.
Moral of the story - you probably don't need connectAdvanced.

@markerikson

This comment has been minimized.

Copy link
Contributor

commented Dec 12, 2018

That, and it seems as if there was some trail of documentation or comments which made it seem as though connectAdvanced was necessary to use forwardRef.

If you do manage to reconstruct exactly what the sequence was that led you to that conclusion, please let us know so we can improve the docs!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.