-1

Hyall

Can you please point out bad practices / mistakes in the code below?

class App extends React.Component {
  constructor(props) {
    super(props);
    this.state = {
      title: "default title"
    };
    this.inputTxt = this.state.title;

    this.myRef = React.createRef();
  }

  componentDidMount() {
    this.myRef.current.value = this.inputTxt;
  }

  handleSubmit = e => {
    e.preventDefault();
    console.log("submitted");
    this.setState({ ...this.state, title: this.inputTxt });
  };

  handleInput = e => {
    this.inputTxt = e.target.value;
  };

  render() {
    return (
      <>
        <div>{this.state.title}</div>
        <form onSubmit={this.handleSubmit}>
          <input
            type="text"
            onChange={this.handleInput}
            ref={this.myRef}
          ></input>
          <button type="submit">Save</button>
          <button type="reset">Reset</button>
        </form>
      </>
    );
  }
}

And some special questions:

  1. is it ok to use this.somevar properties of component class to store variables' values? how to avoid naming collisions?

  2. is it normal to use refs to set input's value?

  3. if I want to set onChange and value bound to reactive variable in one input control, it will freeze? how to gain [(ngModel)] Angular-like control over input element?

Darchan
  • 1
  • 1

1 Answers1

0

It seems like you're over complicating things. I don't see a need for refs here. I don't think setting a class property will trigger a re-render, so this way of managing input might not work at all regardless of it not being a best practice.

Just use state as the value, and update state on change. To keep things flexible, use the input's name as the state key. Something like this:

class App extends React.Component {
  constructor(props) {
    super(props);
    this.state = {
      title: "default title"
    };
  }

  handleSubmit = e => {
    e.preventDefault();
    console.log("submitted");
    // Not sure if thats what you're looking for..
    // Also: no need to do {...this.state, }. setState does a merge, not overwrite
    this.setState({ title: this.state.input1 }); 
  };

  handleChange = e => {
    // Use e.target.name as the computed property name, 
    // so it can be used for infinite number of inputs
    this.setState({[e.target.name]: e.target.value});
  };

  render() {
    return (
      <>
        <div>{this.state.title}</div>
        <form onSubmit={this.handleSubmit}>
          <input
            type="text"
            name="input1" // Give it a unique name for setting state
            value={this.state.input1} // Specify the value instead of using a ref
            onChange={this.handleChange}
          ></input>
          <button type="submit">Save</button>
          <button type="reset">Reset</button>
        </form>
      </>
    );
  }
}

Here is the link to the react docs on refs. The primary they recommend use-cases are:

  • Managing focus, text selection, or media playback.
  • Triggering imperative animations.
  • Integrating with third-party DOM libraries. Which I don't believe apply, here. So I wouldn't recommend using them here.
Brian Thompson
  • 13,263
  • 4
  • 23
  • 43