2

I'm new to react, wanted to ask if this piece of code is good practice, because I have a feeling I'm doing something wrong but not sure what.

I have a main class component which has an array of packages which consists of width, height etc.

I'm passing this packages array as props to another functional component where I want to update these values. Currently my implementation looks like this:

<Card pck={pck} key={pck.packageId}/>



export default function Card(props) {

const widthProperties = useState(0);
props.pck.width = widthProperties[0]
const setWidth = widthProperties[1];

<input type="number" id={props.pck.packageId} className="form-control"
                               value={props.pck.width} 
                               onChange={(e) => setWidth(parseInt(e.target.value))}
                               placeholder="Width" required/>
}

It works correctly, but as I said, I believe that I'm not using the useState with props correctly. Could someone explain what is wrong here? Because 3 lines of code to update props' state looks strange for me.

7
  • 3
    You never directly mutate props. This const [width, setWidth] = useState(0); should be in the parent and passed down to the children like <Card width={width} setWidth={setWidth} key={pck.packageId}/>, then used in the Card like <input value={props.width} onChange={props.setWidth} />.
    – GG.
    Apr 2, 2021 at 22:02
  • @GG. Ok, understood. Could you help me figure it out. I'm iterating through packages array from class component each time creating a <Card /> component. So how would that look then? this.state.packages.map(pck => <Card width={pck.width} ? /> How would the setWidth function look like? If i have my packages in classes state object.
    – developer1
    Apr 2, 2021 at 22:34
  • 1
    Yes that seems fine. The setWidth will probably take an id as argument so you can find the package you want to modify. You will have something like setWidth = (width, packageId) => { this.setState({ packages: this.state.packages.map(pck => pck.id == packageId ? { ...pck, width } : pck) }) }.
    – GG.
    Apr 2, 2021 at 22:54
  • 1
    Then yes it should be pck.packageId == packageId. Are you sure you passing a correct packageId to setWidth? You can add a console.log(width, packageId) inside setWidth to see what's going on.
    – GG.
    Apr 2, 2021 at 23:17
  • 1
    @GG. please submit your answer so I could mark this issue as resolved :)
    – developer1
    Apr 9, 2021 at 18:14

3 Answers 3

4

You never directly mutate props like you do here: props.pck.width = widthProperties[0].

To have a correct data flow, width and setWidth should be in the parent component and passed down to the children, so that the children can update their width by calling setWidth.

So, since the parent is a class component, you will have something like:

class CardsList extends Component {
  state = {
    packages: []
  }

  componentDidMount() {
    fetch('api.com/packages')
      .then(response => response.json())
      .then(result => this.setState({ packages: result.items }))
  }

  setWidth = (width, packageId) => {
    this.setState({
      packages: this.state.packages.map(
        pck => pck.packageId === packageId ?  { ...pck, width } : pck
      )
    })
  }

  render() {
    return (
      <div className="cards-list">
        {this.state.packages.map(pck => (
          <Card pck={pck} setWidth={this.setWidth} key={pck.packageId}/>
        ))}
      </div>
    )
  }
}

And a Card component like:

const Card = ({ pck, setWidth }) => (
  <input value={pck.width} onChange={e => setWidth(e.target.value, pck.packageId)} />
)
-1

It's common to destructure the value and setter function from useState like so:

[value, setValue] = useState(initialValue);

From what I gather from your question, the props.pck.width is the initial value of the input, so you may do something like this:

[width, setWidth] = useState(props.pck.width);

<input type="number" id={props.pck.packageId} className="form-control"
value={width} 
onChange={(e) => setWidth(parseInt(e.target.value))}
placeholder="Width" required/>
-1

You don't use useState like that. useState returns an array of two things:

  • The variable you are going to use
  • A function which is used to change that variable

So in you case it should look like this:

const [widthProperties, setWidthProperties] = useState({}); //Here you can either pass an empty object as an initial value or any structutre you would like.

setWidthProperties(props.pck.width); //Or whatever you want to set it to.

Remember never to change the variable manually. Do it only through the function useState gives you.

Your Answer

Reminder: Answers generated by Artificial Intelligence tools are not allowed on Stack Overflow. Learn more

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.