Preact: Text input ignores value attribute

[vadimdemedes]

Hey Jason, thanks for Preact, I love it!

I noticed a compatibility issue in Preact, when it behaves differently to React. The case is a simple component with input[type="text"] inside. No event listeners, no updates, just a static component:

class TextInput extends Component {
  render({ value }) {
    return <input type="text" value={ value } />;
  }
}

In React, input field ignores user input and remains empty (as it should). In Preact, input field accepts all user input.

Here are minimal test cases to compare Preact to React:

• Preact - http://www.webpackbin.com/NJ_pr2zYW
• React - http://www.webpackbin.com/EJthihMYW

_Press "Save" button at the top to run the code._

Additional info:
Preact version: 5.6.0

Thanks!

[developit]

Thanks @vdemedes! This seems to be a side effect of Preact not overriding the browser's event handling. I'm not sure how to proceed though, since that's one of the things that Preact intentionally eschews in order to remain as small and unintrusive as possible. Maybe this is something that ends up needing to go into preact-compat?

[vvo]

I have a related issue. I have selenium tests running on IE11 and trying to fill a controlled input like this:

        <input
          onChange={e => this.setState({from: e.target.value})}
          ref={type}
          type="number"
          value={this.state.from}
        />

If I send "123", it will fill with "1". 2 and 3 being ignored. This seems to be a timer problem somewhere. Filling the input manually inside IE11 works, just the selenium test doesn't work. This could be a selenium issue but I believe something is wrong in the event ordering between onChange/setState/state update/render in preact here (I am using preact-compat, not sure this is relevant).

Sorry this is vague I agree and I cannot reproduce the issue programmatically (test), only when using IE11 and selenium. Still the react build works in the tests, always.

[developit]

@vvo does changing onChange to onInput cause all 3 characters to be accepted?

[vvo]

@vvo does changing onChange to onInput cause all 3 characters to be accepted?

I tried that and the result was the same, sometime 113 would be 11 or 13 but most of the time it ends up being 1 in the input.

Maybe if we wrote a test simulating very close browser manipulation we would see it. I cannot refrain from adding this:

When I read "Batching of DOM updates, debounced/collated using setTimeout(1) (can also use requestAnimationFrame)" on the preact website. I think: "WARNING WARNING WARNING!" :D

[developit]

There are lots of tests in the suite that would fail if state batching didn't work as sync or tight async though - I've actually never had a bug show up relating to that (not to say it's impossible).

Try turning off batching:

import { options } from 'preact';

options.debounceRendering = f => f();

// or:

options.syncComponentUpdates = true;

Here's a fiddle that shows the effects of batching on controlled inputs via onChange:
https://jsfiddle.net/developit/ufvm71L5/

[vvo]

I've actually never had a bug show up relating to that (not to say it's impossible).

Do you also run the test suite on IE11?

[vvo]

Maybe that's just an issue of selenium not triggering the change events in a very clean way on the input on IE11. And somehow react solved it in their code maybe using some custom code. It's not worth it diving more in it (we solved it by changing the test a bit and adding a comment), sorry for interrupting

[developit]

Ah good point, I skipped over the browser specific bit. I just pulled it up (and modified it to fire synthetic events) in IE11 on SauceLabs and it seemed to work:
https://jsfiddle.net/developit/ufvm71L5/19/

[developit]

Thought: I could have preact-compat's Component#setState() be synchronous? Right now it's just preact's (ref).

[vvo]

I am all in for trying that in my selenium test, I will try to do so tomorrow just to see if that would do it. but setState is asyn in react right? So this should already be good.

[developit]

It is batched yeah, though the way preact batches will collect DOM updates as well.

[developit]

I believe any remaining issue here has been corrected by the fix for #327. Anyone able to confirm?

[developit]

I'm going to mark this as resolved for now, since the value pinning idea is unlikely to be feasible in Preact's size and the other DOM state issue seems to be resolved. Shout if you think this should be re-opened!

[thinkloop]

Text input ignores value attribute

Preact basically has react's uncontrolled inputs model. This means the dom maintains the input's state, and input values are free to change as the user interacts with them, regardless of what new props are coming in.

Most of the time this is what is needed. Sometimes, however, it is important to be able to override existing values based on new props. For example, clearing a form after successful submission. To accomplish this, we can bind inputs to state, and use setState when new props come in. For example:

export default class Input extends Component {
  constructor({ inputValue }) {
    super();
    this.stateFromProps(inputValue);
  }

  componentWillReceiveProps({ inputValue }) {
    this.stateFromProps(inputValue);
  }

  render(props, { inputValue }) {
    return (
      <input value={ inputValue } />
    );
  }

  stateFromProps = (inputValue) => {
    this.setState({ inputValue });
  }
}

@developit thoughts?

[developit]

@thinkloop that's mostly true, though as of 6.3.0+ (including the just released 7.1.0) re-rendering an input will diff against its DOM state value. The case where inputs are uncontrolled is only when you don't trigger a re-render in response to input. I've found this is relatively uncommon, but let me know if you have a case where you're seeing it cause issues. React does a lot of work to prevent inputs from accruing state at all (preventing input that isn't propagated into state through event handlers), but for a minimalist library like Preact that is just too much surface area.

I'm happy to discuss though, just it comes down to tradeoffs (like many things haha). I've found it best to re-render form fields in response to input anyway, which turns them into proper controlled inputs.

[thinkloop]

the case where inputs are uncontrolled is only when you don't trigger a re-render in response to input

You're right! :-P

How does this work? How does preact know if a re-render is coming and to therefore control the input?

For example, how would the OP's demo be modified so that the input is controlled:

const { h, render, Component } = require('preact');

class TextInput extends Component {
  render({ value }) {
    return (
      <input type="text" value={ value } />  
    );
  }
}

render(<TextInput value="" />, document.body);

[developit]

It works because the diff doesn't compare against the previous Virtual DOM tree, it compares against the DOM. If the input's DOM state is out of sync with the tree, rendering pushes the value out to the DOM.

[bodazhao]

struggling with how to make a controlled component in Preact...

import { h, Component } from "preact"

class TextInput extends Component {
  state = {
    input: ""
  }
  render({}, { input }) {
    return (
      <input value={input} />
    )
  }
}

In React, <input /> value would be empty even when the user types, how could we do this in Preact? For example, I'm trying to limit user's input so no spaces allowed...

[lukeed]

Attach an onkeydown listener. If it's not a space, update the state. Updating the state will then update your input value.

[deadlyicon]

I dont know if this is the same problem but I ran into a weird case where my <input/> was not respecting the given value. I'm using preact-8.2.7 and preact-compat-3.18.0.

import React, { Component, PropTypes } from 'react'

import ErrorMessage from 'components/ErrorMessage'

import './index.sass'

export default class TextInput extends Component {
  static propTypes = {
    disabled: PropTypes.bool,
    className: PropTypes.string,
    onChange: PropTypes.func,
    onInput: PropTypes.func,
    onClick: PropTypes.func,
    type: PropTypes.string,
    name: PropTypes.string,
    value: PropTypes.string,
  }

  componentDidMount(){
    if (this.props.autofocus) this.input.focus()

    // THIS IS THE HACK THAT WORKED :(
    setTimeout(() => {
      if (this.input.value !== String(this.props.value)) this.forceUpdate()
    }, 10)
  }

  onChange = () => {
    if (this.props.onChange) this.props.onChange(this.input.value)
  }

  onInput = () => {
    if (this.props.onInput) this.props.onInput(this.input.value)
  }

  onClick = event => {
    if (this.props.onClick) this.props.onClick(event)
  }

  focus(){
    return this.input.focus()
  }

  render(){
    const {
      className = '',
      type = 'text',
      ...props
    } = this.props
    props.type = type
    props.onChange = this.onChange
    props.onInput = this.onInput
    delete props.onClick
    props.autoCapitalize = "none"
    props.autoCorrect = "off"

    return <div
      className={`TextInput ${className}`}
      disabled={props.disabled}
      onClick={this.onClick}
    >
      <div className="TextInput-wrapper">
        <input
          ref={input => { this.input = input }}
          {...props}
        />
      </div>
    </div>
  }
}

I hope this helps someone :(

[developit]

@deadlyicon are you providing a value prop to the input? (can't tell from the code sample since props is dynamic)

[deadlyicon]

@developit yes I am providing both a value and an onChange prop. I suspect this new bug behavior is a recent change in chrome but that's just a guess.

[dg-sserrano]

Still the same error in 10.4.0.

[JoviDeCroock]

@dg-sserrano this is an old issue, could you make a new one with a reproduction. Template: https://codesandbox.io/s/epic-snowflake-w7rwi?file=/src/index.js

[deadlyicon]

can you please link the new issue here so i can follow it too

[JoviDeCroock]

@deadlyicon I think the issue you posted above should be fixed in Preact X (gave it a quick glance)