Undo behavior in controlled input doesn't work as expected

[oliviertassinari]

Do you want to request a feature or report a bug?

Report a bug

What is the current behavior?

1. Load https://codesandbox.io/s/material-demo-8wgfs.
2. Focus the first input.
3. Type a.
4. Press Tab to focus the second input.
5. Type a.
6. Use Ctrl/Cmd+Z or Edit->Undo to undo these two changes.

Notice that the uncontrollable input change is reverted Notice that the controllable input change is not reverted

What is the expected behavior?

uncontrollable & controllable inputs behave identically.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

React 16.12.0 Chrome 78.0.3904.108 macOS 10.14.6

First reported in https://github.com/mui-org/material-ui/issues/18545.

[kunukn]

I could not reproduce it. Maybe I am doing something wrong. On mac and windows 10, Chrome browser. I used the tabbing to select previous input.

---

Docs:

Controlled component. Where the source of truth comes from React. https://reactjs.org/docs/forms.html#controlled-components

Uncontrolled component Where the source of truth is in the DOM. https://reactjs.org/docs/uncontrolled-components.html

---

The Undo/Redo is applied from onChange event. I have created a simple Controlled vs Uncontrolled demo where I console.log what happens when Undo/Redo is applied. https://codesandbox.io/s/reactjs-controlled-vs-uncontrolled-input-1jpju

[oliviertassinari]

@kunukn Thank you for looking at it. In what I can benchmark, the set state in the focus handler is the origin of the issue (I have used Cmd+Z).

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

[stale[bot]]

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!

[ernestostifano]

@kunukn Thank you for looking at it. In what I can benchmark, the set state in the focus handler is the origin of the issue (I have used Cmd+Z).

I came across this issue while working on a form validation library (I was actually testing it with some of your input components, along with others) and it seems that any state change triggered outside of the change event will break undo/redo behaviour (except on Firefox, where it breaks even for the most simple controlled input).

I've even tried to make my changes by manually dispatching a change event after modifying the native input value via its prototype value setter, obtaining the same results.

The only interesting thing that I've discovered is that in some browsers (I remember WebKit based ones), the focus event (when undoing an input different than the currently focused one) will give you the correct undone value under event.target.value (for example, an empty string), but then it will be immediately overridden with the previous value (however, sometimes the cursor jumps to the correct undone position, even if value does not change).

In some very rare, likely random cases, I've found that previous input value even gets duplicated when trying to undo. Like "Test" turns into "TestTest" after undo.

I think React inputs implementation should be revised. And while at it, please consider amending #9567.

[andrelmchaves]

@oliviertassinari @ernestostifano Could you confirm this is still an issue?

[punkpeye]

Just ran into this issue.

Is there a workaround?

[mbrowne]

I wonder if this was one of the factors considered by some authors of frameworks and form libraries that are now favoring uncontrolled components rather than controlled - that seems to be a trend. I'm sure the general trend is unrelated to this undo/redo issue, but now that I know about it, it would be one of the things I would consider when deciding whether to use a controlled or uncontrolled component in the first place. Of course I realize that there are some use cases for which a controlled component makes a lot more sense and requires less code, and it would be great if this issue was just fixed by default somehow when using controlled components.