Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changing defaultValue doesn't re-render input, causes hidden state in the db #4101

Closed
togakangaroo opened this issue Jun 11, 2015 · 8 comments

Comments

@togakangaroo
Copy link

I posted this on StackOverflow but the dearth of answers makes me think it might be a bug.

Given the following simple code

const {createFactory, createClass, DOM: { label, input, button }} = React;

const tester = createFactory(createClass({
  render() {
      return label({}
               ,`Name: ${this.props.name}`
               ,input({defaultValue: this.props.name})
               ,button({onClick: this.changeName}, "Change")
             )
  },
  changeName() {
    this.setProps({name: "Wilma"})
  }
}) )

React.render(tester({name: "Fred"}), document.querySelector('body'))

clicking the button doesn't re-render the input and it still says "Fred" even though this.props.name is "Wilma".

Demo

@sophiebits
Copy link
Collaborator

The answer you got on Stack Overflow is correct. defaultValue only specifies the value when the input is initially created; if you want it to remain updated you need to use value and handle onChange events correctly.

We don't update when defaultValue changes because otherwise you have two sources (the user's input and your component props) trying to control the value, and it wouldn't be clear what should happen when they conflict. Instead, we force you to think about and write out the actual behavior you want.

@togakangaroo
Copy link
Author

But hold on, in that case I'm going to argue that this is a bug as you can end up with a scenario where the current HTML is not stateless.

The specific situation where I ran into this - a grid, and when double clicking on a row I popped open a dialog form to edit the row items. I noticed that my input values were stuck to the first value I opened even though everything else changed. How can this be correct behaviour? The whole point is that state is not stored in the DOM...

If changing defaultValue doesn't cause the input to re-render then you can pass the exact same model through render twice and get different HTML output.

@togakangaroo
Copy link
Author

Also the docs need to be updated then, as they imply that defaultValue is the correct thing to use if you don't want to lock the value

@sophiebits
Copy link
Collaborator

The whole point is that state is not stored in the DOM...

With defaultValue, the state is stored in the DOM. That's why we don't recommend it. We recommend using controlled components instead. The docs say:

If you wanted to update the value in response to user input, you could use the onChange event.

(http://facebook.github.io/react/docs/forms.html#controlled-components)

Let me know if that's still confusing.

@ghost
Copy link

ghost commented Aug 31, 2016

I posted this on the SO but I'll include it here as well.

I found what seems to be a pretty good solution to this: Use the key prop to force rendering of an entirely new input.

In my particular case, I don't need the input to be controlled with its own onChange prop, as the form surrounding it ultimately controls the state within some store which populates the defaultValue. But the store's state might be asynchronously initialized/updated, and in which case the defaultValue should be updated. So here is a condensed version of my particular case:

import React, { PropTypes } from 'react';
import { Form } from 'provide-page';

const GatherContact = ({
  classes,
  onSubmit,
  movingContactName,
  movingContactEmail,
  movingContactPhone,
  userName,
  userEmail,
  userPhone
}) => (
  <Form onSubmit={onSubmit}>
    <div className={classes.GatherContact}>
      <h2 className={classes.GatherHeading}>
        How can we contact you?
      </h2>

      <input
        type="text"
        className={classes.GatherContactInput}
        placeholder="Name"
        name="movingContactName"
        key={`movingContactName:${movingContactName || userName}`}
        defaultValue={movingContactName || userName}
        required={true}
      />

      <input
        type="email"
        className={classes.GatherContactInput}
        placeholder="Email"
        name="movingContactEmail"
        key={`movingContactEmail:${movingContactEmail || userEmail}`}
        defaultValue={movingContactEmail || userEmail}
        required={true}
      />

      <input
        type="tel"
        className={classes.GatherContactInput}
        placeholder="Phone"
        name="movingContactPhone"
        key={`movingContactPhone:${movingContactPhone || userPhone}`}
        defaultValue={movingContactPhone || userPhone}
        required={true}
      />

      {userName
        ? undefined
        : (
          <input
            type="password"
            className={classes.GatherContactInput}
            placeholder="Password"
            name="userPassword"
            required={true}
            autoComplete="new-password"
          />
        )
      }
    </div>
  </Form>
);

GatherContact.propTypes = {
  classes: PropTypes.object.isRequired,
  onSubmit: PropTypes.func.isRequired,
  movingContactName: PropTypes.string.isRequired,
  movingContactEmail: PropTypes.string.isRequired,
  movingContactPhone: PropTypes.string.isRequired,
  userName: PropTypes.string.isRequired,
  userEmail: PropTypes.string.isRequired,
  userPhone: PropTypes.string.isRequired
};

export default GatherContact;

@Venryx
Copy link

Venryx commented Dec 18, 2017

My solution is to add a wrapper component, which uses local state when the user is editing the value, and only triggers the onChange function once focus is lost (or equivalent).

Here's an example, for the npm "rc-slider" component:

import RCSlider from "rc-slider";

export class Slider extends Component
		<{min: number, max: number, step: number, value: number, delayChangeTillDefocus?: boolean, onChange: (val: number)=>void},
		{editedValue: number}> {
	slider: RCSlider;
	render() {
		let {value, delayChangeTillDefocus, onChange, ...rest} = this.props;
		let {editedValue} = this.state;
		return (
			<RCSlider ref={c=>this.slider = c} {...rest} value={editedValue != null ? editedValue : value} onChange={val=> {
				if (delayChangeTillDefocus) {
					this.setState({editedValue: val});
				} else {
					onChange(val);
					this.setState({editedValue: null});
				}
			}}
			onDefocus={val=> {
				if (delayChangeTillDefocus && onChange) {
					onChange(val);
					this.setState({editedValue: null});
				}
			}}/>
		);
	}
}

@mborysenko
Copy link

mborysenko commented Feb 20, 2020

The whole point is that state is not stored in the DOM...

With defaultValue, the state is stored in the DOM. That's why we don't recommend it. We recommend using controlled components instead. The docs say:

If you wanted to update the value in response to user input, you could use the onChange event.
(http://facebook.github.io/react/docs/forms.html#controlled-components)

Let me know if that's still confusing.```

It is still confusing.
As I remember the value property of input is mutable but defaultValue is not. You can only set defaultValue from outside of the element. So for me it makes more sense to use defaultValue if you want to use controlled component. I can see issue with using value: when user choose something in select and using value prop to populate chosen value through the state(controlled way) value is added twice to the element.
In controlled way you kind of rerender the component, basically initialize it with new options every time they change, which should update component as it was created from the scratch. Then the concept that says that component is updated every time props are changed is not always true.

@mborysenko
Copy link

I posted this on the SO but I'll include it here as well.

I found what seems to be a pretty good solution to this: Use the key prop to force rendering of an entirely new input.

In my particular case, I don't need the input to be controlled with its own onChange prop, as the form surrounding it ultimately controls the state within some store which populates the defaultValue. But the store's state might be asynchronously initialized/updated, and in which case the defaultValue should be updated. So here is a condensed version of my particular case:

import React, { PropTypes } from 'react';
import { Form } from 'provide-page';

const GatherContact = ({
  classes,
  onSubmit,
  movingContactName,
  movingContactEmail,
  movingContactPhone,
  userName,
  userEmail,
  userPhone
}) => (
  <Form onSubmit={onSubmit}>
    <div className={classes.GatherContact}>
      <h2 className={classes.GatherHeading}>
        How can we contact you?
      </h2>

      <input
        type="text"
        className={classes.GatherContactInput}
        placeholder="Name"
        name="movingContactName"
        key={`movingContactName:${movingContactName || userName}`}
        defaultValue={movingContactName || userName}
        required={true}
      />

      <input
        type="email"
        className={classes.GatherContactInput}
        placeholder="Email"
        name="movingContactEmail"
        key={`movingContactEmail:${movingContactEmail || userEmail}`}
        defaultValue={movingContactEmail || userEmail}
        required={true}
      />

      <input
        type="tel"
        className={classes.GatherContactInput}
        placeholder="Phone"
        name="movingContactPhone"
        key={`movingContactPhone:${movingContactPhone || userPhone}`}
        defaultValue={movingContactPhone || userPhone}
        required={true}
      />

      {userName
        ? undefined
        : (
          <input
            type="password"
            className={classes.GatherContactInput}
            placeholder="Password"
            name="userPassword"
            required={true}
            autoComplete="new-password"
          />
        )
      }
    </div>
  </Form>
);

GatherContact.propTypes = {
  classes: PropTypes.object.isRequired,
  onSubmit: PropTypes.func.isRequired,
  movingContactName: PropTypes.string.isRequired,
  movingContactEmail: PropTypes.string.isRequired,
  movingContactPhone: PropTypes.string.isRequired,
  userName: PropTypes.string.isRequired,
  userEmail: PropTypes.string.isRequired,
  userPhone: PropTypes.string.isRequired
};

export default GatherContact;

I cannot say that this is good solution, as it disables some features of select, for example. In case with multiple select if you use this WA, and you need select many items, you will have to click as many times as many element you have to choose. Annoying but still can work. This is good WA but not the solution.

dariocravero pushed a commit to viewstools/morph that referenced this issue Feb 20, 2021
Default props wouldn't update (facebook/react#4101 (comment))
when changed during development which causes unexpected behaviours.
Eg, you have a fontSize with a defaul value on a Design System block and change it, before this,
you won't notice the change until the component is mounted again.

It also seems that there’s a proposal to deprecate defaultProps in React
reactjs/rfcs#107
facebook/react#16210
https://www.xspdf.com/resolution/59631680.html.
pwoidke pushed a commit to pwoidke/react-sudoku that referenced this issue Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants