Why does my onChange event for input change multiple state values in an object array?

Question

I am building a user flow to allow users to enter stats for a video game that they played. My react component renders an excel like table where they can manually type in the stats (like an excel spreadsheet where the y-axis is a column of players and the x-axis are the stats for each player.

When I update a particular stat for a particular player, every player's stat value for that particular stat is also changed. For example, if I update Player A with 5 goals every other player on the team will also have 5 goals. This shouldn't happen.

First, I initialize a matchStats object:

initializeMatchStats = (numGames) => {
    const {matchInfo} = this.props;
    const gameArray = new Array(numGames).fill('');

    const matchStats = gameArray.map((game, i) => {
        let statsWithKeys = {};
        let gameInfo = {};

        matchInfo.circuitStats.map(key => {
            statsWithKeys[key.toLowerCase()] = 0
        })

            const players = new Array(matchInfo.playersAllowedInGame).fill({
                stats: statsWithKeys
            }).map(p => {
                return {
                    ...p,
                    dropdownOpen: false,
                    id: Math.random().toString(36)
                }
            })

            gameInfo = {
                players,
                index: i + 1
            }

        return gameInfo
    })
    this.setState({matchStats, numGames, numGamesModalOpen: false, uploadManually: true}, () => console.log('matchStats: ', matchStats))
}

Then, I update the matchStats object as it is changed by the user:

updateStatsManually = (val, player, game, header, e) => {

        e.preventDefault();

        const {matchStats} = this.state;
        // matchStats is an array of games in each match. A game is made up of the players (and their respective stats) in the game.
        const { matchInfo } = this.props;

        const gameToUpdate = matchStats.find(g => g.index === game.index)

        let playerToUpdate = gameToUpdate.players.find(p => p.id === player.id)

        let newStats = playerToUpdate.stats;
        newStats[header] = val;
        playerToUpdate.stats = newStats;
        gameToUpdate.players = [...gameToUpdate.players.filter(p => p.id !== player.id), playerToUpdate]

        this.setState({
            matchStats: [...matchStats.filter(g => g.index !== game.index), gameToUpdate]
        })
    }

Here is the react code block:

<Section>
    {matchStats.find(g => g.index === game).players.map((player, i) => <Row key={i}>
        {!uploadManually && <Column>
            <Input disabled style={{textAlign: 'center'}} placeholder={player.name} />
        </Column>}
        {circuitStats.map((header, i) => <Column key={`${i}-${player.id}`}>
            <Input id={`${i}-${player.id}-${header}`} value={player.stats[header.toLowerCase()]} disabled={!uploadManually} onChange={(e) => updateStatsManually(e.target.value, player, currentGame, header.toLowerCase())} />
        </Column>)}
    </Row>)}
</Section>

I would expect that when I change an input value for one stat for a given player, it only changes the stat value for that particular player. However, it changes it for every player.

I've been messing with this for a while now and struggling to see what I'm doing wrong. I thought it might be related to how inputs are rendered in a .map function but I tried separating a test input and the result was the same. Any help is appreciated!

Can you show where these structures are being created? There must be some aliasing going on. — ggorlen
– ggorlen, Commented Sep 25, 2019 at 20:59
@ggorlen I added the code block where I initialize the matchStats object. Hopefully that helps! — Kyle Pendergast
– Kyle Pendergast, Commented Sep 25, 2019 at 21:12
It helps a lot, thanks. new Array(matchInfo.playersAllowedInGame).fill({ is likely the trouble. .fill just does one object for the whole shebang. Change it to something like fill().map(e => {stats: statsWithKeys})... and let me know if it works. Likely, you'll only ever want to pass primitives into .fill(). — ggorlen
– ggorlen, Commented Sep 25, 2019 at 21:20
That didn't seem to work unfortunately. I changed it to the following: ` const players = new Array(matchInfo.playersAllowedInGame).fill().map(p => { return { stats: statsWithKeys, dropdownOpen: false, id: Math.random().toString(36) } }) ` — Kyle Pendergast
– Kyle Pendergast, Commented Sep 25, 2019 at 21:30
That should be fill().map(e => ({stats: statsWithKeys})); (note the extra parens). So did your change work? The problem is that statsWithKeys is still just one object as well, so that's a problem. You're going to have to initialize that object inside the map as well. Basically, any time there is just one object being created, it'll be aliased, so there are multiple aliasing problems to resolve. — ggorlen
– ggorlen, Commented Sep 25, 2019 at 21:37

ggorlen · Accepted Answer · 2019-09-25 22:02:10Z

The first problem is the following line:

const players = new Array(matchInfo.playersAllowedInGame).fill({
    stats: statsWithKeys
}) // ... etc ...

By passing an object into Array#fill, every element is populated with a reference to that one single object. The stack snippet shows this clearly in the following minimal reprex:

const arr = Array(4).fill({age: 11});
console.log(JSON.stringify(arr));
arr[0].age = 42;
console.log(JSON.stringify(arr));
console.log(arr);

Change the line to

const players = new Array(matchInfo.playersAllowedInGame).fill().map(e => ({
  stats: statsWithKeys
})) // ... etc ...

that is, return a different object for each index in the array to refer to. Or, since you're chaining this with another .map call, you can combine them into one.

Here's a fix to the above reprex:

const arr = Array(4).fill().map(e => ({age: 11}));
console.log(JSON.stringify(arr));
arr[0].age = 42;
console.log(JSON.stringify(arr));
console.log(arr);

The second problem (similar to the first) is that there's an object let statsWithKeys = {}; which is aliased for each player by the above map function. We need a different statsWithKeys instance for each player.

Full re-write that moves initialization of statsWithKeys into the map. I used some dummy data to make it reproducible and eliminated unnecessary temporary variables:

const numGames = 3;
const matchInfo = {
  circuitStats: ["foo", "bar"], 
  playersAllowedInGame: 4
};

const matchStats = Array(numGames).fill().map((game, i) => {
  return {
    index: i + 1,
    players: Array(matchInfo.playersAllowedInGame).fill().map(p => {
      return {
        stats: matchInfo.circuitStats.reduce((a, e) => {
          a[e.toLowerCase()] = 0;
          return a;
        }, {}),
        dropdownOpen: false,
        id: Math.random().toString(36) // this is not actually safe! use a uuid
      }
    })
  }
});

matchStats[0].players[0].stats.foo = 42; // try setting something
console.log(matchStats);

Another major problem, as indicated above, is that Math#random is not safe for generating unique ids. If you have a lot of data (or even if you don't and the code is run often enough), you'll wind up with unintended behavior. See the links in this paragraph for the latest fixes.

Michael Bugglin · Accepted Answer · 2019-09-25 22:14:47Z

Let's say you have a limit of 5 players. In this case, this:

         const players = new Array(matchInfo.playersAllowedInGame).fill().map(p => {
                return {
                   stats: statsWithKeys,
                   dropdownOpen: false,
                   id: Math.random().toString(36)
                }
            })

isn't creating 5 'statsWithKeys' as you'd expect, but rather 5 references to the same 'statsWithKeys'.

The best way to fix this is to use a spread operator directly on the object itself:

         const players = new Array(matchInfo.playersAllowedInGame).fill().map(p => {
                return {
                   stats: { ...statsWithKeys },
                   dropdownOpen: false,
                   id: Math.random().toString(36)
                }
            });

Collectives™ on Stack Overflow

Why does my onChange event for input change multiple state values in an object array?

2 Answers 2

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related