Fix controlled input values out of sync with dom #1929
Conversation
| @@ -122,5 +122,6 @@ function setProperty(dom, name, value, oldValue, isSvg) { | |||
| * @private | |||
| */ | |||
| function eventProxy(e) { | |||
| return this._listeners[e.type](options.event ? options.event(e) : e); | |||
| this._listeners[e.type](options.event ? options.event(e) : e); | |||
There was a problem hiding this comment.
Do we need to still capture the return value from the handler and return it?
There was a problem hiding this comment.
I don't think we do. We attach eventProxy like a standard event handler and those don't return anything:
dom.addEventListener("input"; e => {
// No need to return anything here
});| @@ -122,5 +122,6 @@ function setProperty(dom, name, value, oldValue, isSvg) { | |||
| * @private | |||
| */ | |||
| function eventProxy(e) { | |||
| return this._listeners[e.type](options.event ? options.event(e) : e); | |||
| this._listeners[e.type](options.event ? options.event(e) : e); | |||
| if (this._lastValue!=this.value) this.value = this._lastValue; | |||
There was a problem hiding this comment.
This logic will apply for any event added to an input element, right? So if I add onFocus, I'll need to also make it a controlled input in order for the input to work? Just checking if I understand how this works.
There was a problem hiding this comment.
ahhhh good catch. Maybe this should guard and only apply for onChange/onInput? Though right now it means this works with either, and guards against state mutations in focus/blur (maybe never happens?)
|
I'll leave this open as I want to check if this PR leads to cursor jumps or something. |
|
Just checked, this solution leads to cursor jumping for all inputs 😢 Just jumping in this masked scenario is fine, because react does the same, but this PR breaks every controlled input in that regard :S Slapping a defer via a Promise tick around it, solves the problem in my test cases, but I'm not sure if that's the right solution. |
Fixes #1899 .
Adds
+15 B