0

I want to generate a 16-length array of random prizes using prizes array that is passed as a prop in Board component, and display them.

prizes array -

[
    {
        prizeId: 1,
        name: 'coupon',
        image: 'img/coupon.svg',
    },
    {
        prizeId: 2,
        name: 'gift card',
        image: 'img/gift-card.svg',
    },
    // more prizes
]

In Board.js -

const Board = ({ prizes }) => {
    const [shuffledPrizes, setShuffledPrizes] = useState(null)

    useEffect(() => {
        setShuffledPrizes(shuffleArray(populatePrize(16, prizes)))
    }, [prizes])

    return (
        <div>
            {
                shuffledPrizes && shuffledPrizes.map((prize) => (
                    <Prize
                        key={prize.id}
                        prize={prize}
                    />
                ))
            }
        </div>
    )
}

In populatePrize function, I have to add id to use as React key because already existed prizeId can't be used, as prizes will be duplicated -

import { nanoid } from 'nanoid'

const populatePrize = (noOfBlock, prizeArray) => {
    const arrayToPopulate = []

    let index = 0
    for (let i = 0; i < noOfBlock; i += 1, index += 1) {
        if (index === prizeArray.length) {
            index = 0
        }
        arrayToPopulate.push({
            id: nanoid(),
            prizeId: prizeArray[index].prizeId,
            name: prizeArray[index].name,
            image: prizeArray[index].image,
        })
    }

    return arrayToPopulate
}

Is using useState and useEffect necessary here? Because, I don't think generating an array and shuffling it is a side effect, and I can just use a variable outside of Board function like -

let shuffledPrizes = null

const Board = ({ prizes }) => {
    if (!shuffledPrizes)
        shuffledPrizes = shuffleArray(populatePrize(16, prizes))
    }

    return (
        <div>
            {
                shuffledPrizes.map((prize) => (
                    <Prize
                        key={prize.id}
                        prize={prize}
                    />
                ))
            }
        </div>
    )
}

But, with that way, every <Board /> component references and display the same shuffledPrizes array, not randomly for each Board component like I want.

Reusing Board is not a requirement, but I read in React docs about components being pure functions and I don't think mine is one. I am also confused in when to use a variable outside or inside of a component, and when to use state.

Although my question might be about using useEffect, I want to learn how to improve this code in proper React way.

Htet Oo Wai Yan
  • 113
  • 3
  • 12
  • Usually every non reactive variable that never gets mutated can be safely put outside of the component. It's usefull when you need to pass it to effects dependency for example – Reifocs Sep 15 '22 at 09:52

2 Answers2

1

This in indeed not a good use case of useEffect.

Effects are an escape hatch from the React paradigm. They let you “step outside” of React and synchronize your components with some external system like a non-React widget, network, or the browser DOM. If there is no external system involved (for example, if you want to update a component’s state when some props or state change), you shouldn’t need an Effect. Removing unnecessary Effects will make your code easier to follow, faster to run, and less error-prone.

You can shuffle the array when you pass it trough props.

const BoardContainer = () => <div>
    <Board prizes={shuffleArray(populatePrize(16, prices))}/>
    <Board prizes={shuffleArray(populatePrize(16, prices))}/>
</div>

You can also use the lazy version of useState that is only evaluated during the first render

const Board = ({prizes}) => {
    const [shuffledPrizes,] = useState(() => shuffleArray(populatePrize(16, prizes)))

    return (
        <div>
            <ul>
                {
                    shuffledPrizes && shuffledPrizes.map((prize) => (
                        <Prize
                            key={prize.id}
                            prize={prize}
                        />
                    ))
                }
            </ul>
        </div>
    )
}
Reifocs
  • 704
  • 3
  • 16
  • `prizes` gets reshuffled when `BoardContainer` re-renders. I'm making like a match to win game. After user has clicked a pair of `Prize`, the states in `BoardContainer` will update, like `hasWon` or `chancesLeft`. – Htet Oo Wai Yan Sep 15 '22 at 10:46
  • lazy useState should work though, as long as you Board component does not get recreated – Reifocs Sep 15 '22 at 10:59
  • if the prices ref is stable, you could also memo the shuffleArray shuffleArray(populatePrize(16, prices)), [prices])}/> – Reifocs Sep 15 '22 at 11:04
  • Lazy useState does work. But, I'm wondering, isn't state only used for data that changes? Or, is it okay to used here in this kind of scenario? – Htet Oo Wai Yan Sep 15 '22 at 11:15
  • The docs said to use useMemo only for optimization, not for semantic. Sorry, I'm referencing the documentation here because I don't have experience with useMemo. – Htet Oo Wai Yan Sep 15 '22 at 11:19
  • Using state without the setter function is basically like using a ref, it's often used to initialize object only once per component lifecycle.. Your right about useMemo, should avoid it if possible – Reifocs Sep 15 '22 at 11:33
0

Your prizes are given in props, so they can potentially be updated ? By a fetch or something like that.
In that case, you can :


cont defaultArray = []; // avoid to trigger useEffect at each update with a new array in initialization 

const Board = ({ prizes = defaultArray }) => {
    const [shuffledPrizes, setShuffledPrizes] = useState([])

    useEffect(() => {
       if(prizes.length) {
           setShuffledPrizes(shuffleArray(populatePrize(16, prizes)));
       }
    }, [prizes]);

    return (
        <div>
            {
                shuffledPrizes.map((prize) => (
                    <Prize
                        key={prize.id}
                        prize={prize}
                    />
                ))
            }
        </div>
    )
}

If you do :

const Board = ({ prizes }) => {
    const shuffledPrizes = shuffleArray(populatePrize(16, prizes))

    return (
        <div>
            {
                shuffledPrizes.map((prize) => (
                    <Prize
                        key={prize.id}
                        prize={prize}
                    />
                ))
            }
        </div>
    )
}

populatePrize and shuffleArray will be called at each render. Maybe it could works if your only props is prices and you use React.memo. But it's harder to maintain, I think.

Making a variable out of your component like that, will not let your component listen to this variable modifications. You can do this for constants.

Each render you test !shuffledPrizes so when it will be filled once, your variable will be filled too and your component will render correctly. But if you change prizes, shuffledPrizes will not be updated. It's not a good practice.

With a different condition, you can continue to update your out component variable listening to prop changes that trigger a render. But useEffect is the better way to listen if your prop changes.

In the code you post, shuffledPrizes can be null, so you should put a condition before calling .map()

My self, I would call the suffle function in the parent that store it in is state, to store it directly with shuffling and not calling shuffle function at a wrong rerender.

Geo Daz
  • 214
  • 1
  • 11