preactjs / preact

⚛️ Fast 3kB React alternative with the same modern API. Components & Virtual DOM.
https://preactjs.com
MIT License
36.57k stars 1.95k forks source link

Uncontrolled input #1899

Open Crysp opened 5 years ago

Crysp commented 5 years ago

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

JoviDeCroock commented 5 years ago

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} />;
dwelle commented 5 years ago

@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?

JoviDeCroock commented 5 years ago

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.

dwelle commented 5 years ago

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).

JoviDeCroock commented 5 years ago

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

Crysp commented 5 years ago

@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

JoviDeCroock commented 5 years ago

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

dwelle commented 5 years ago

@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

Crysp commented 5 years ago

@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

dwelle commented 5 years ago

@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

Crysp commented 5 years ago

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

EirikFA commented 4 years ago

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

greasysock commented 4 years ago

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} />
}
18choi18 commented 3 years ago

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;
}} />
ChenKS12138 commented 3 years ago

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} />;
}
fabiospampinato commented 2 years ago

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.

nullpaper commented 2 years ago

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.

Dev1OptimusTecno commented 1 year ago

any updates?

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

d-eisenga commented 6 months ago

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.

scarf005 commented 2 months ago

also for signals, the workaround A) prevents rendering optimization (which is a killer feature of signals) B) stops working as soon as computed values are used:

const text = signal({})
const renderedText = computed(() => text.value.v)

const ControlledInput = () => {
    const onInput = e => {
        text.value = ({ v: e.currentTarget.value.replace(/[^\d]/, '') })
    }
    return <input value={renderedText} onInput={onInput} />
}

which feels like a serious footgun.

repl link

vipexv commented 1 hour ago

It's been years, how is this still not fixed?