-1

Anyone know why this isn't generating any components?

sArray is a 2d array with a structure like a box [[1,2],[4,4]], but it's 5x5

      {
        sArray.map( (array) => {
         console.log('inthemap');
         array.forEach(element => {
          console.log('intheEach');
          return (
            <div>
            <S ticketValue={element}> </S>
            </div>
          )
         });
          
         
            
        })
      }

When I run the page the Console logs the InTheMap and then the InTheEach 5 times, and it does this 5 times. Meaning I'm making 25 S components, but they do not show up.

When I remove the forEach, the S component does show up.

Mark
  • 15
  • 6

1 Answers1

-1

There are two issues I see above:

  1. You are missing a return statement for the inner loop
  2. You should be using map instead of forEach since you are returning
{
  sArray.map((array) => {
    // you forget to return this
    return array.map(element => {
      return (
        <div>
          <S ticketValue={element}> </S>
        </div>
      )
    })
  })
}

Also quick note that this has a runtime complexity of O(n^2) which may be ok for smaller arrays, but will slow performance exponentially as they grow in size.

You may want to move this from outside the render method and compute with useMemo to prevent unnecessary re-renders: https://reactjs.org/docs/hooks-reference.html#usememo

// compute this value only once (or add dependencies for when it should update)
const myExpensiveRender = useMemo(() => {
  return sArray.map((array) => {
    return array.map(element => {
      return (
        <div>
          <S ticketValue={element}> </S>
        </div>
      )
    }
},[])

return (
  <>
    {myExpensiveRender}
  </>
)
Asleepace
  • 3,466
  • 2
  • 23
  • 36
  • I'm not sure why someone downvoted, but this did not work for me. – Mark May 27 '22 at 20:35
  • 1
    @Mark ah wait I see another issue as well, you should be using `map` instead of `forEach` – Asleepace May 27 '22 at 20:35
  • 1
    Yup you are right, I tried two maps before but was missing the return then as well, as it kept saying it wasn't a function. Thanks @Asleeppace :) – Mark May 27 '22 at 20:40
  • The array will be hardlocked at 5x5, so I think the complexity should be OK? Anyway would I use the memo as ------ const memoizedValue = useMemo(() => { sArray.map((array) => { return array.map(element => { return (
    ) }) }) }) ------ And then just pop in the{memoizedValue} in my actual JSX? I realize the formatting is horrible sorry.
    – Mark May 27 '22 at 21:02
  • @Mark for small values it should be ok, but it also depends on how many times the parent is re-rendering. It's a good thing to get in the habit of, just updated the answer! – Asleepace May 27 '22 at 21:08
  • @Asleepace it doesn't have runtime complexity of O(n^2). It renders exactly as many items as there are items total, the fact that some items are wrapped in inner array doesn't change anything. Complexity is O(n) (if comlpexity term is even applicable to render). Also this is not a good use for useMemo because it doesn't account for changing `sArray`, and if you try to add it to dependencies list, it will break as sArray is not guaranteed to keep referential equality, e.g. if it comes from props – Max May 28 '22 at 15:36
  • @Max simply wrong, if you have an array of arrays that is re-rendered literally in the render method that is the definition of O(n^2) and furthermore every time this component is rendered this needs to be recomputed. I mentioned the dependency list in my comments, and like I said if it needs to be rendered once leave it as it is, otherwise add dependencies. This is textbook useMemo show me a better example I’ll wait – Asleepace May 28 '22 at 15:41
  • @Asleepace I suggest you read on time complexity first https://en.wikipedia.org/wiki/Time_complexity. You are mixing many things into one here. If rendering one item takes `x` amount of time, then rendering 20 items takes `20x` amount of time, regardless if these items are in one flat array `[1,2,3,...20]` or in nested arrays `[[1],[2,3],[4,[5,...20]]]` - you still make as many iterations as there are items. That is the definition of O(n) – Max May 28 '22 at 15:57
  • For proper usecases of useMemo you can refer to react doc, useMemo is used to compute expensive data, not to avoid rendering parts of the component. Let alone the fact that rendering array is not an expensive operation by any means – Max May 28 '22 at 15:58
  • @Asleepace did you make an assumtion for O(n^2) just based on whether maps are nested or not? Maybe you are referring to stack depth but not to time complexity? Wherever you add new elements, time taken to render those elements will be proportional to the number of elements (e.g. if rendering 10 elements takes 10ms, then rendering 10000 elements takes 10000ms). You still iterate each item once, not less and not more - this is what you should take into account talking about time complexity, not whether or not your data is nested – Max May 28 '22 at 16:05
  • @Max I see your point, but if it is the case that the array is literally a 1:1 mapping of arrays to arrays, you should be adding a new answer describing how this can be accomplished with a singular array then. Either way it’s in the render method, let’s say the parent component has an event listener and updates on a regular time interval, if the data doesn’t change should we re-render this “flat” list each time? Absolutely not. – Asleepace May 28 '22 at 16:09
  • @Asleepace your answer is wrong in 2 parts - suggesting time complexity is O(n^2) and suggesting to use useMemo to optimize rendering. Regarding the first one, if you are still unsure and refuse to read on time complexity, you can do an experiment in your browser - calculate time it takes your browser to render 100 items (let's say 1ms) and then 10000 items. If complexity is O(n) then 10000 items are rendered in approximately 100 ms, but if it's O(n^2) then in 1000000ms which is obviously absurd. – Max May 28 '22 at 16:23
  • Regarding rendering - yes, it's perfectly fine to render items from arrays. In fact it's the only way React renders elements. And since we've established it's O(n) it's not a complex operation at all. If you try to optimize with useMemo you need to account for array coming from props (in that case its reference likely changes every render, even if data itself doesn't change, and useMemo will still recalculate every render, being useless) [...] – Max May 28 '22 at 16:23
  • [...] If you want to work around that then you will need to define ways to figure out whether items have really changed (like trying to put `JSON.stringify(array)` in useMemo deps) and you get to the point when trying to decide whether useMemo should recalculate your result or reuse a previous one is a more expensive operation than just not using useMemo at all – Max May 28 '22 at 16:23