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 object -

[
    {
        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.

CodePudding user response：

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>
    )
}

CodePudding user response：

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.