saveData method saves twice-CodePudding

I am building a React app that includes one separate component for CRUD functionality of Products and another separate component for CRUD functionality of Suppliers.

I am using the same saveData method for both components (the Create functionality of CRUD.. that is triggered when the User presses Save after filling in the input fields of Product or Supplier). The saveData method is located in a central ProductsAndSuppliers.js file that is available to both the Products and Supplier components.

In both of the Product & Supplier components, there is a table showing the Products or Suppliers already present as dummy data. I made a button at the bottom of each page to add a new Product or Supplier... depending on which tab the user has selected on the left side of the screen (Product or Supplier).

Since I am using the same saveData method in both cases, I have the same problem whenever I try to add a new Product or Supplier to each respective table after filling out the input fields. My new Product or Supplier is added.. but twice and I can't figure out why.

I have tried using a spread operator to add the new item to the collection but am having no success:

  saveData = (collection, item) => {
    if (item.id === "") {
      item.id = this.idCounter  ;
      this.setState((collection) => {
        return { ...collection, item }
      })
    } else {
      this.setState(state => state[collection]
        = state[collection].map(stored =>
          stored.id === item.id ? item : stored))
    }
  }

Here is my original saveData method that adds the new Product or Supplier, but twice:

  saveData = (collection, item) => {
    if (item.id === "") {
      item.id = this.idCounter  ;
       this.setState(state => state[collection]
         = state[collection].concat(item));
    } else {
      this.setState(state => state[collection]
        = state[collection].map(stored =>
          stored.id === item.id ? item : stored))
    }
  }

my state looks like this:

this.state = {
  products: [
    { id: 1, name: "Kayak", category: "Watersports", price: 275 },
    { id: 2, name: "Lifejacket", category: "Watersports", price: 48.95 },
    { id: 3, name: "Soccer Ball", category: "Soccer", price: 19.50 },
  ],
  suppliers: [
    { id: 1, name: "Surf Dudes", city: "San Jose", products: [1, 2] },
    { id: 2, name: "Field Supplies", city: "New York", products: [3] },
  ]
}

CodePudding user response：

There are issues with both of your implementations.

Starting with the top one:

// don't do this
this.setState((collection) => {
   return { ...collection, item }
})

In this case, collection is your component state and you're adding a property called item to it. You're going to get this as a result:

{
  products: [],
  suppliers: [],
  item: item
}

The correct way to do this with the spread operator is to return an object that represents the state update. You can use a computed property name to target the appropriate collection:

this.setState((state) => ({
    [collection]: [...state[collection], item]
  })
)

_{* Note that both this and the example below are using the implicit return feature of arrow functions. Note the parens around the object.}

In the second code sample you're

mutating the existing state directly which you should not do.
returning an array instead of a state update object.

// don't do this
this.setState(state =>
  state[collection] = state[collection].concat(item)
);

Assignment expressions return the assigned value, so this code returns an array instead of an object and I'd frankly be surprised if this worked at all.

The correct implementation is the same as above except it uses concat instead of spread to create the new array:

this.setState(state => ({
    [collection]: state[collection].concat(item)
  })
);