Try to make 2048 game in React. Data in the state is mutated when it should not have been

Question

This is my code repo https://github.com/540376482yzb/2048game_react

This is my live demo https://objective-fermat-0343a3.netlify.com/

The current state of the game only goes left and right.

However it generates new number on left arrow key pressed when no move was made. The identified issue is this.state.gridData is flipped before I run this.diffGrid(grid) therefore it will always return true and added new number.

The suspects on this issue is :

    // issue ====>  flipGrid function is mutating this.state.gridData
flipGrid(grid) {
    return grid.map(row => row.reverse())
}

Or

    //slide left
            if (e.keyCode === 37) {
                //issue ===> state is flipped on left arrow key pressed
                copyGrid = this.flipGrid(copyGrid).map(row => this.slideAndCombine(row))
                copyGrid = this.flipGrid(copyGrid)
            }

Can someone tell me where did I do wrong to cause the state mutation?

import React from 'react'
import './App.css'
import Grid from './Grid'
class App extends React.Component {
 constructor(props) {
  super(props)
  this.state = {
   gridData: [[0, 0, 0, 0], [0, 0, 0, 0], [0, 0, 0, 0], [0, 0, 0, 0]]
  }
 }
 initGame() {
  let grid = [...this.state.gridData]
  grid = this.addNumber(grid)
  grid = this.addNumber(grid)
  this.setState({
   gridData: grid
  })
 }

 addNumber(grid) {
  const availableSpot = []
  grid.map((rowData, x) =>
   rowData.map((data, y) => {
    if (!data) availableSpot.push({ x, y })
   })
  )
  const randomSpot = availableSpot[Math.floor(Math.random() * availableSpot.length)]
  grid[randomSpot.x][randomSpot.y] = Math.random() < 0.2 ? 4 : 2
  return grid
 }
 slide(row) {
  const newRow = row.filter(data => data)
  const zerosArr = Array(4 - newRow.length).fill(0)
  return [...zerosArr, ...newRow]
 }

 combine(row) {
  let a, b
  for (let i = 3; i > 0; i--) {
   a = row[i]
   b = row[i - 1]
   if (a === b) {
    row[i] = a + b
    row[i - 1] = 0
   }
  }
  return row
 }

 slideAndCombine(row) {
  row = this.slide(row)
  row = this.combine(row)
  return row
 }

 diffGrid(grid) {
  let isDiff = false
  for (let i = 0; i < grid.length; i++) {
   for (let j = 0; j < grid.length; j++) {
    if (grid[i][j] != this.state.gridData[i][j]) {
     isDiff = true
    }
   }
  }
  return isDiff
 }

 // issue ====>  flipGrid function is mutating this.state.gridData
 flipGrid(grid) {
  return grid.map(row => row.reverse())
 }

 componentDidMount() {
  this.initGame()
  let copyGrid = [...this.state.gridData]
  window.addEventListener('keyup', e => {
   if (e.keyCode === 37 || 38 || 39 || 40) {
    //slide right
    if (e.keyCode === 39) {
     copyGrid = copyGrid.map(row => this.slideAndCombine(row))
    }
    //slide left
    if (e.keyCode === 37) {
     //issue ===> state is flipped on left arrow key pressed
     copyGrid = this.flipGrid(copyGrid).map(row => this.slideAndCombine(row))
     copyGrid = this.flipGrid(copyGrid)
    }

    // Line 89 issue==>>>>>> gridData in the state
    console.table(this.state.gridData)

    // diffGrid compares copyGrid with this.state.gridData
    if (this.diffGrid(copyGrid)) {
     copyGrid = this.addNumber(copyGrid)
     //deepCopy of gridData
     console.table(copyGrid)

     this.setState({
      gridData: copyGrid
     })
    }
   }
  })
 }

 render() {
  // Line 103 ===>>>> gridData in the state
  console.table(this.state.gridData)
  return (
   <div className="App">
    <main className="centerGrid" id="game">
     <Grid gridData={this.state.gridData} />
    </main>
   </div>
  )
 }
}

export default App

Olivier Wilkinson · Accepted Answer · 2018-03-30T22:10:48.497

row.reverse() will return the reversed array, but it will also mutate the row array. Have a look at the MDN docs for reverse. So, because map doesn't copy the elements before iterating over them, your flipGrid function will mutate the grid passed to it.

To ensure you don't mutate the original grid you could do the following:

flipGrid(grid) {
    const gridCopy = grid.slice()
    return gridCopy.map(row => row.reverse())
}

or a bit more concisely:

flipGrid(grid) {
    return grid.slice().map(row => row.reverse())
}

or if you wanted to use the spread operator:

flipGrid(grid) {
    return [...grid].map(row => row.reverse())
}

As for why this.state.gridData is mutated: array spreading is actually a shallow copy, so gridCopy is still referencing this.state.gridData's row arrays, because of this you are still mutating this.state.gridData when you mutate with flipGrid. If you stop mutating entirely this wouldn't matter, but it is better to deep copy at the beginning of the event handler rather than in componentDidMount so this can't happen in the first place.

you can either use lodash's cloneDeep function or map over gridCopy and shallow copy those also (using the spread operator or slice)

window.addEventListener('keyup', e => {
    if (e.keyCode === 37 || 38 || 39 || 40) {
        let gridCopy = this.state.gridData.map((row) => row.slice())
        etc...
    }
}

The argument I passed in to `flipGrid(copyGrid)` is a deep copy array of `this.state.gridData` so it should not have affect the original data, correct? — ZhouBing Yang, Mar 30 '18 at 20:12
I used spread operator `[...]` instead of `.slice()` to do the trick — ZhouBing Yang, Mar 30 '18 at 20:21
@ZhouBingYang Hey, I updated my answer, if anything doesn't make sense just comment again :) — Olivier Wilkinson, Mar 30 '18 at 21:07
Thank you. Yes I didn't realize using [...] on nested array is doing shallow copy. I learned a lot today. Thank you mr. — ZhouBing Yang, Mar 30 '18 at 21:30

Try to make 2048 game in React. Data in the state is mutated when it should not have been

1 Answers1