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

dispatch with setState and with nested connect break user input #1896

Closed
1 task done
reshetnev-gb opened this issue Apr 13, 2022 · 8 comments
Closed
1 task done

dispatch with setState and with nested connect break user input #1896

reshetnev-gb opened this issue Apr 13, 2022 · 8 comments

Comments

@reshetnev-gb
Copy link

What version of React, ReactDOM/React Native, Redux, and React Redux are you using?

  • React: 17.0.2
  • ReactDOM/React Native: 17.0.2
  • Redux: 4.1.2
  • React Redux: 7.2.6

What is the current behavior?

Demo: https://codesandbox.io/s/react-redux-nested-connect-break-input-n4pr3j. See console for rendering details.

Steps:

  1. Remove focus from the input element.
  2. Set the cursor to the center of the input element.
  3. Enter a character.

Actual behavior: The cursor jumps to end of the input element.
Expected behavior: The cursor stays in place after entering character into input element.

What is the expected behavior?

The cursor stays in place after entering character into input element.

Which browser and OS are affected by this issue?

OS: macOS, Windows. Browser: Google Chrome.

Did this work in previous versions of React Redux?

  • Yes
@reshetnev-gb
Copy link
Author

We are gradually rewriting the application to react hooks and also want to use react-redux with hooks. But also we have class components wrapped with ReactRedux.connect.

Is it possible to fix this problem?

@reshetnev-gb
Copy link
Author

reshetnev-gb commented Apr 13, 2022

I tried to fix this issue and wrote my ownConnect using hooks from react-redux and it worked.

Tell me please what could be the disadvantages of such a solution and why it is not used by default? Because of rendering the whole tree?

Demo with ownConnect: https://codesandbox.io/s/react-redux-nested-connect-break-input-forked-899swl?file=/src/own-connect.js

@markerikson
Copy link
Contributor

Can you clarify what you mean by "this worked with other versions of React-Redux"? Are you saying it worked with a literal prior version of the library with the exact same application code? Or that it was the same library version, but different app code?

FWIW, your version of connect is missing a lot of critical behavior that is in the real implementation.

@markerikson
Copy link
Contributor

At first glance I'm not sure what exactly is causing this, and unfortunately I don't have time to look into this myself any time soon.

There's a long relevant thread over in the React repo at facebook/react#955 (comment) discussing cursor jumping.

Overall, we specifically recommend not putting form state directly into Redux. So, my main advice here would be to change this to just be component state, and dispatch an action with the updated value after the user is done with the input.

@reshetnev-gb
Copy link
Author

Can you clarify what you mean by "this worked with other versions of React-Redux"?

react-redux@6.0.0

Are you saying it worked with a literal prior version of the library with the exact same application code? Or that it was the same library version, but different app code?

it was the same app code with react-redux@6.0.0. Example: https://codesandbox.io/s/react-redux-nested-connect-break-input-forked-mnlcrg

@markerikson
Copy link
Contributor

markerikson commented Apr 13, 2022

Yeah, there are many differences in how v6 and v7 work internally. v6 relied on React's top-down state updates, where v7 went back to external subscriptions - see #1177 and https://blog.isquaredsoftware.com/2018/11/react-redux-history-implementation/ for details.

and tbh I'm shocked you've been on v6 this long - that version had a flawed architectural approach and has been dead since spring 2019. was there a particular reason you held off on upgrading to v7?

I'd be curious to see if v8 beta improves this, but it feels like it's related to fundamental architectural assumptions that we can't change.

So, my main suggestions atm are A) try v8 RC, and B) don't keep form state in Redux.

@reshetnev-gb
Copy link
Author

and tbh I'm shocked you've been on v6 this long - that version had a flawed architectural approach and has been dead since spring 2019. was there a particular reason you held off on upgrading to v7?

Previously, all of our code was built on classes. But now we are actively writing new components using hooks.
And we understand that react-redux@6.0.0 will not work well with concurrent mode and with new react features in future.

@reshetnev-gb
Copy link
Author

Thanks for answer and suggested solutions. We will try to fix our application code.

@timdorr timdorr closed this as completed Apr 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants