preact icon indicating copy to clipboard operation
preact copied to clipboard

Uncontrolled input

Open Crysp opened this issue 6 years ago • 21 comments

Same logic with different libraries works different.

React - ✅ https://codesandbox.io/s/react-controlled-input-egxgu

Preact - ❌ https://codesandbox.io/s/preact-controlled-input-kib3p

Crysp avatar Aug 27 '19 11:08 Crysp

This is an inherent flaw to the inner workings of vdom and html for now, I'll see if I can find a solution to this problem but for now I can't see one yet.

Let me draw the situation:

  1. for your first three keystrokes you pass in a different state
  2. the fourth keystroke will return the same state meaning the component won't rerender
  3. the HTMLelement however does the default behavior and appends a new keystroke

This can be prevented by doing the following, this is a workaround for now and I'll take a look if we can fix this without patching the event system:

return <input value={value} onInput={onChange} maxLength={3} />;

JoviDeCroock avatar Aug 28 '19 10:08 JoviDeCroock

@JoviDeCroock what does React do in this case? Binds keydown event and prevents it, whenever an input is controlled (there's value={value} on it)? Can't Preact do the same?

dwelle avatar Aug 28 '19 11:08 dwelle

React ships big code bundles of event patching for these scenario's. I don't think adding a maxLength is that bad of a solution though. HTML traditionally offers all solutions but interpreting them from JS makes you overwrite all native events code.

JoviDeCroock avatar Aug 28 '19 11:08 JoviDeCroock

Right. Preventing keydown wouldn't actually help because e.target.value couldn't be used anymore and you'd need to set up workarounds around e.which and that opens a whole new can of worms and you end up with what React does. So no easy fix I can see either :/.

One possibility is to re-render even on same-state change when that state controls input value (not sure how easy would be to determine that).

dwelle avatar Aug 28 '19 11:08 dwelle

That's not possible without a flag or static code analysis (which is not possible with a runtime library)

JoviDeCroock avatar Aug 28 '19 11:08 JoviDeCroock

@JoviDeCroock this is simple example... what if i want to format received value?

event.target.value.replace(/[^0-9]g/, '')

https://codesandbox.io/s/preact-controlled-input-2-1olep

Crysp avatar Aug 28 '19 12:08 Crysp

@Crysp nothing is stopping you from using maxLength for that too if I understand the problem you're trying to show well enough

JoviDeCroock avatar Aug 28 '19 12:08 JoviDeCroock

@JoviDeCroock he's right --- in his example it should prevent writing numbers which it fails to do so due to this bug. maxLength doesn't have a say in this.

Similarly, if you wanted to allow only letters, you wouldn't be able to do so:

https://codesandbox.io/s/preact-controlled-input-2-5gwc4

dwelle avatar Aug 28 '19 13:08 dwelle

@JoviDeCroock as far as i understand, set doesn't use deep comparison for prev and next value.

i want to write only numbers into state. if i input abc and then input 1 state, value of field resets to 1. for first three characters set received empty string, compare default state and they are equal. but value of field still uncontrolled.

i think this not logical behaviour

Crysp avatar Aug 28 '19 13:08 Crysp

@Crysp it seems the only workaround is to leverage the behavior that useState does shallow comparison, and use a non-primitive to force re-render:

const ControlledInput = () => {
  const [data, setData] = useState({ value: "" });

  const onChange = event =>
    setData({ value: event.target.value.replace(/[^a-z]/gi, "") });

  return <input value={data.value} onChange={onChange} />;
};

https://codesandbox.io/s/preact-controlled-input-2-rvghz

dwelle avatar Aug 28 '19 13:08 dwelle

@dwelle thx, i'll use your variant until bug will be fixed

Crysp avatar Aug 28 '19 15:08 Crysp

I am having a similar issue with value simply being ignored. Setting it still allows any input.

React: https://codesandbox.io/s/gallant-frog-eeul9 Preact: https://codesandbox.io/s/nifty-brahmagupta-0z15g

EirikFA avatar Feb 06 '20 16:02 EirikFA

I am having issues with this as well, in the meantime I made this hook for creating controlled inputs:

import * as React from 'react'

/** preact/compat controlled input fix */

/**
 * Preact/compat hook for fixing controlled inputs
 * @param value string value of the state to bind to input
 * @param onChange callback method invoked when changing input
 */
export const useControlledInput = (value:string, onChange: (newValue: string) => void) => {
  const inputRef = React.useRef<HTMLInputElement>()

  React.useEffect(()=>{
    if(inputRef.current && value !== inputRef.current.value){
      inputRef.current.value = value || ''
    }
  },[value])

  React.useEffect(()=>{
    if(inputRef.current){
      // @ts-ignore - ts types are incorrect for onchange
      inputRef.current.oninput = (ev) => onChange(ev.target.value)
    }
  }, [onChange])

  return [
    /** Bind to ref for input */
    (el: HTMLInputElement|null) => {
      if(!inputRef.current && el){
        // @ts-ignore
        el.oninput = (ev) => onChange(ev.target.value)
        inputRef.current = el
        if(el.value !== value) { el.value = value || '' }
      }
    }
  ]
}

example comp:

import React, { useState } from 'react'
import { useControlledInput } from '../hooks'

const TestInput = () => {
  const [ controlledText, setControlledText ] = useState('')
  const [ controlledTextRef ] = useControlledInput( controlledText, (changes) => {
  // Do some validation
  setControlledText(changes)
} )
  return <input ref={controlledTextRef} />
}

greasysock avatar Feb 13 '20 16:02 greasysock

I solved this issue by forcibly synchronizing the value of the input dom to the state.

<input value={x} onChange={evt => {
  const slicedValue = evt.target.value.slice(0, 3);
  setX(slicedValue);
  evt.target.value = slicedValue;
}} />

18choi18 avatar Sep 24 '20 14:09 18choi18

I also encountered the same problem and I solved it like this

interface IInput {
  value: number | string;
  onInput: (e: any) => void;
  id: string;
  type: "text";
}

export default function Input(props: IInput) {
  const handleInput = useCallback(
    (event) => {
      const newValue = event.target.value;
      event.target.value = props.value;
      const newEvent = {
        ...event,
        target: { ...event.target, value: newValue },
      };
      props.onInput && props.onInput(newEvent);
    },
    [props.value]
  );

  return <input {...props} onInput={handleInput} />;
}

ChenKS12138 avatar Oct 26 '20 11:10 ChenKS12138

IMO this is a pretty significant issue, compatibility with React goes out of the window if stuff like this is broken.

According to the docs controlled components actually work, IMO it should be updated to note that they don't actually work (at least for inputs, I don't know about the rest), especially considering how this issue is more than 2 years old, having the docs provide wrong information just makes people lose a bit of sanity.

fabiospampinato avatar Nov 02 '21 00:11 fabiospampinato

As @fabiospampinato says, the problem here is that the docs are flat-out misleading. From the docs https://preactjs.com/guide/v10/forms/#controlled--uncontrolled-components:

"The component doesn't manage the value itself there, but something else higher up in the component tree. ... Generally, you should try to use Controlled Components at all times."

// Controlled, because Preact manages the input's value now
<input value={someValue} onInput={myEventHandler} />;

Yeah, except if you actually try and and control your input you get completely undocumented behaviour that works nothing like a "controlled input" is expected to. Nothing on that page even hints at the fact that Preact doesn't actually control your input.

If I type "1234" into an example "controlled input" that replaces numbers, you get "1234" rickrolled:

<input
  value={someValue}
  onInput={e => {
    const numbers = /[0-9]/g
    setSomeValue(e.target.value.replace(numbers, ''))
  }}
/>

// Lol this isn't controlled at all, nothing is replaced

Preact just needs to explain this in the docs.

nullpaper avatar Dec 07 '21 14:12 nullpaper

any updates?

this is preventing me from switching from react to preact because it is a fundamental part of my program.

Dev1OptimusTecno avatar Aug 01 '23 18:08 Dev1OptimusTecno

I'd like to know the status as well. Forcibly calling e.target.value = x on every input event moves the element's cursor to the end of the text, which is horrible UX, but there seems to be no other way to ensure that the input element doesn't become uncontrolled.

d-eisenga avatar Feb 25 '24 09:02 d-eisenga