Inferno: Controlled input, cursor jumps to the end

Created on 18 Jan 2017  ·  25Comments  ·  Source: infernojs/inferno

Observed Behaviour

In controlled inputs after a change cursor jumps at the end of the field

Expected Current Behaviour

Inferno should preserve cursor position on input change

https://jsfiddle.net/ua8hqfjf/3/

Inferno and inferno-component version 1.2
macOS
Chrome

Investigation bug

Most helpful comment

I can confirm the issue. Moreover, when using inferno-redux, it is not easily worked around by using setStateSync, as inferno-redux handles setting the state: https://github.com/infernojs/inferno/blob/master/src/redux/connect.ts#L261-L283

        handleChange() {
            if (!this.unsubscribe) {
                return;
            }
            const storeState = this.store.getState();
            const prevStoreState = this.state.storeState;

            if (pure && prevStoreState === storeState) {
                return;
            }
            if (pure && !this.doStatePropsDependOnOwnProps) {
                const haveStatePropsChanged = tryCatch(this.updateStatePropsIfNeeded, this);
                if (!haveStatePropsChanged) {
                    return;
                }
                if (haveStatePropsChanged === errorObject) {
                    this.statePropsPrecalculationError = errorObject.value;
                }
                this.haveStatePropsBeenPrecalculated = true;
            }
            this.hasStoreStateChanged = true;
            this.setState({ storeState });
        }

All 25 comments

Confirmed its bug: https://jsfiddle.net/69z2wepo/67824/ (React, same codebase)

This looks something we should fix and release 1.2.1

This works if you use synchronous setState (setStateSync)
https://jsfiddle.net/ua8hqfjf/9/

Ultimately setState should work same way with React to not confuse people. Changing that is anyway larger task than simple patch and won't be included for 1.2.1 @trueadm is this doable?

@Havunen this change would basically need to set a flag on a component with something like isCommiting, i.e. when a mount, patch, or unmount is occurring. During this phase, the setState should be async always, otherwise sync?

Not sure if this related but if I have input field without event but with default value in template like this:
html <input type="text" value={this.state.value}/>
let say default value empty this.state.value = ''
Typing to that input field does not work. No text entering. I don't see that render get called as well. That would explain why text not appearing, this looks like 2 way binding or something which is not good.

Sorry @kurdin, this actually isn't related. You're setting the value to '' on every render, and without any handlers attached (eg, onInput), you have no way of setting the new state.value.

Please see this example for a reference on how to update an input's value via state.

You may also hop into our Slack & a lot of people will be able to help you out!

@lukeed ok, I understand that state value did not change, because I don't have onInput event handler with setState call for value. But why/how value of input set to '' on text enter/type? Render method did not get called (I am sure because I used console.log inside render to check so). How come when I type value sets to '' again without render method? This does not sound right. What if I want to setState of value onBlur event?
This won't work:
html <input type="text" value={this.state.value} onBlur={this.handleBlur}/>
Sorry if this is wrong ticket.

P.S looks like React has same behavior. Strange

@kurdin by using value you are telling Inferno to take ownership of this DOM element. Meaning, whatever you do on the browser is no longer important because you're not handling it with an onInput event listener – so it gets discarded.

If the value you have set is this.state.value and you don't actually change this.state.value, then why should the input ever change too?

This is exactly how it should work :)

@trueadm Thanks for explaination. I thought in React like approach UI views controlled by render method. Set new value in setState should call render and then input gets new value from state. Simple. Now it is turns out that inputs and some form elements have it own behavior linked to value. What if I want set default value of input on mount only then check errors and setState onBlur? Looks like it is not possible. I can not set regular value attribute to anything.

feature request to add defaultValue like react: https://github.com/infernojs/inferno/issues/743

@kurdin as per you other issue, you should be able to use defaultValue to do what you're describing.

@trueadm confirmed that defaultValue works with inferno.

@Havunen @trueadm
I found the bug in a different scenario. Basically my input is not controlled by the state. onInput dispatches an action to a redux store and the component receives back the value as a prop. So value={this.props.value}. This works with react and it's useful to make the whole app react on input change. A simple fiddle to emulate this scenario:

https://jsfiddle.net/ua8hqfjf/16/

In my case I can solve the problem using defaultValue so the input is not controlled.
But I can think about other scenarios where using defaultValue is not an option: reducer can reject the change for any reason and send back the old value as a prop.

The fiddle shows another problem I found with defaultValue in textarea. Opened another issue for this.

@fmarsoni using setStateSync fixes cursor jumping issue:
https://jsfiddle.net/ehm2evbx/

Fixing setState is too complex for me, it requires most likely @trueadm 's input

I can confirm the issue. Moreover, when using inferno-redux, it is not easily worked around by using setStateSync, as inferno-redux handles setting the state: https://github.com/infernojs/inferno/blob/master/src/redux/connect.ts#L261-L283

        handleChange() {
            if (!this.unsubscribe) {
                return;
            }
            const storeState = this.store.getState();
            const prevStoreState = this.state.storeState;

            if (pure && prevStoreState === storeState) {
                return;
            }
            if (pure && !this.doStatePropsDependOnOwnProps) {
                const haveStatePropsChanged = tryCatch(this.updateStatePropsIfNeeded, this);
                if (!haveStatePropsChanged) {
                    return;
                }
                if (haveStatePropsChanged === errorObject) {
                    this.statePropsPrecalculationError = errorObject.value;
                }
                this.haveStatePropsBeenPrecalculated = true;
            }
            this.hasStoreStateChanged = true;
            this.setState({ storeState });
        }

The work regarding this has started:

https://github.com/infernojs/inferno/pull/785

But this is difficult issue to get right. Sorry for the delay.

Thanks, that is some nice news to wake up to for a change

There is a related issue with radio buttons: given controlled radio buttons, when you click the last one in a group it updates only after next render.
You can try it here: https://codepen.io/dinalt/pen/ZLxqxm

initial PR to fix this behavior:
https://github.com/infernojs/inferno/pull/785

I have been wonder how could we write test case for this cursor jumping to avoid regression? Help wanted.

Maybe check input.selectionStart / input.selectionEnd on componentDidUpdate.

I'm always using the stable version (it seems to be 1.2.2 now), but this bug is causing some headaches to me, so I'd like to know if it's fixed in one of these new release candidate versions.

It should be fixed but there are no tests for it

We need tests for this before launching Inferno 1.5

Added manual tests (examples/compat) and found few issues where cursor jumped when using setState. Those will be fixed in next release 1.5

Closing as 1.5 has been released! 👍

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Havunen picture Havunen  ·  3Comments

alexichepura picture alexichepura  ·  5Comments

mohammedzamakhan picture mohammedzamakhan  ·  3Comments

Elliot-Evans-95 picture Elliot-Evans-95  ·  4Comments

brenr picture brenr  ·  3Comments