Box's properties not updated within a for loop

[Xample]

Hi, I'm having some issues which seems to be related to the react caching

codesandbox

I created 2 components, on the left and on the right, respectively:

• Stripes which is the code I do think it should work but the rendering is wrong
• StripesExpected which is a non optimized code but which rendering is correct

Both components have a "count" property which can be changed using the upper left hand side corner's text input.

Each component display a count of lines for a given height of 1. The higher count, the thinner the strokes.

However, when I reduce the count of lines using the input, the component on the left will NOT update the line's thickness. The component on the right works as expected. For instance, using 5 lines (count = 5):

Here is the code of the broken component:

import { range } from "lodash";
import { Vector3, Color3 } from "@babylonjs/core";
import { useEffect, useState } from "react";

export const Stripes = function Stripe(props: { count: number }) {
  const [height, setHeight] = useState(1 / props.count);
  useEffect(() => {
    setHeight(1 / props.count);
  }, [props.count]);

  return (
    <>
      {range(props.count).map((x, i) => (
        <box
          name={`box-${i}`}
          key={`${i}`}
          width={1}
          height={height}
          depth={0.001}
          position={new Vector3(0, i * height, 0)}
          rotation={new Vector3(0, 0, 0)}
        >
          <standardMaterial
            name={`mat-${i}`}
            diffuseColor={i % 2 ? Color3.Black() : Color3.White()}
            specularColor={Color3.Black()}
          />
        </box>
      ))}
    </>
  );
};

I'm giving a key key={${i}} to identify the boxes. When the count changes, react will load a former box from its cache. This is the expected behavior, however the height property of the loaded cached component is not updated as it should (despite I'm storing the height within a state).

To force this behavior, I had to use a unique key like key={${i}${props.count}} and I removed the useState and useMemo optimization which, as the key is caching everything, are now useless.

I suspect this to be a bug, or how would you do, considering that key={${i}${props.count}} is a hack ?

[brianzinn]

Works as expected, but can see how that is not obvious. You sort of figured it out on the one side. What is happening is that constructor arguments when they change do not rebuild a new instance currently. If you wanted to trigger that behaviour then you could have a key like:

<box key={`${i}-${height}`} ... />

This is called when the object is created: https://doc.babylonjs.com/typedoc/modules/BABYLON#CreateBox-2 It is not called when those "constructor" properties change (from the factory method)

Then the reconciler operates on the mesh that is returned, so props like position can be updated, but then not the original constructor arguments. When you pass the constructor arguments into the key then they will re-generated. I realize that's not ideal, but currently that's how the renderer works.

Does that make sense and work for you?

[Xample]

it does now, and I can remember to have seen mutable and immutable props (if I could name them so). A possible workaround in my case would be to either keep the loop key as is, or to create my box which would be a regular box + a transformNode to change its height.

I agree that it's a little confusing, but it makes sense.

I realize that's not ideal

A possible way could be to have one property dedicated to the inner babylonJS object's constructor, another (bad) solution could be to recreate that inner object if one of those properties change. A smooth approach could be to have a debug mode which would print a message in the console if someone changes a constructor property after the inner object to be constructed (defined). Something like a useAssertPropertyChanged

[Xample]

Okay, so the treejs alternative is using an args property. In short if the args property changes, the whole object is reconstructed (but it is not supposed to change…)

[brianzinn]

closing from inactivity, but also it looks resolved. kindly re-open if you have more questions.