Best practice for accessing current state in a handler

[verekia]

Loving this library so far! I would like to ask what your recommendation would be to access a current state inside a handler. With normal React DOM, each re-render recreates the handlers, but not with react-babylonjs, which keeps the variables in a closure from the first render. I guess my question is a more general follow-up of #139.

import { useState, useRef } from 'react'

import { useClick } from 'react-babylonjs'

const Counter = () => {
  const [count, setCount] = useState(0)
  const ref = useRef(null)

  console.log(count) // Correctly logs 0, 1, 2, 3...

  useClick(() => {
    console.log(count) // Always logs 0. How to access the current count here?
    setCount(n => n + 1) // Ideally this would also just be setCount(count + 1)
  }, ref)

  return <box ref={ref} name="counter" size={10} />
}

export default Counter

Thank you.

[brianzinn]

hi @verekia - glad you are liking the library. I think the primitive being brought in via closure is just a drawback of javascript. Did you consider putting the variable in a dependency array for the callback? Maybe you can share your actual use case - typically what I do is a useRef and then the current will be available in the callback, but not typically would I combine that with a useState as it gets a bit repetitive. If you are using hooks from this library then I may need to revisit if your use case turns out to be convenient or easier to use.

[verekia]

Thank you for your response. I tried using useCallback but I get the same result:

import { useState, useRef, useCallback } from 'react'

import { useClick } from 'react-babylonjs'

const Counter = () => {
  const [count, setCount] = useState(0)
  const ref = useRef(null)

  console.log(count) // Correctly logs 0, 1, 2, 3...

  const handleClick = useCallback(() => {
    console.log(count) // Always logs 0. How to access the current count here?
    setCount(n => n + 1) // Ideally this would also just be setCount(count + 1)
  }, [count])

  useClick(handleClick, ref)

  return <box ref={ref} name="counter" size={10} />
}

export default Counter

My real use-case is that I am making a city-building type of game where you can place buildings on a grid of a map. The user clicks on the type of building they want to make, and then click on a cell of the grid to place that building there. The type of building currently being placed is held in a global Redux-like store (via the Zustand library), and each Cell is subscribed to it via a hook.

The simplified Cell.tsx component looks like this:

const Cell = ({ x, y }) => {
  // This variable is initially null when the game loads, then 'house'
  // after the user clicks on the house button for instance.
  const buildingBeingPlaced = useStore(s => s.buildingBeingPlaced)

  const ref = useRef(null)

  console.log(buildingBeingPlaced) // null, then 'house'

  useClick(() => {
    if (buildingBeingPlaced) {
      addBuildingToMap({ type: buildingBeingPlaced, x, y }) // But buildingBeingPlaced is always null here
    }
  }, ref)

  return (
      <plane ... />
  )
}

If the state was entirely held in the Cell component, I could figure something out with refs, but it comes from outside of the component.

I am no expert at Babylon.js but it seems to me that to behave like "normal React", react-babylonjs should update the event handlers at every render? (by detaching them and re-attaching the new ones I suppose). That's what React does on DOM elements. In this kind of situation, there are no closure problems in normal React, and that keeps the code lightweight and declarative instead of imperative with refs. That's the core strength of React in my opinion.

With the DOM the performance cost of updating event listeners is negligible, but I don't know if that would be the case with Babylon elements.

[brianzinn]

I think even in "normal React" that stale closures have always been a problem. I do understand the issue you are having - did you have a solution in mind? The source is here: https://github.com/brianzinn/react-babylonjs/blob/master/packages/react-babylonjs/src/hooks/utilityHooks.ts#L132

It looks like you can put useStore inside the useClick hook if it's just to retrieve the state, but then it wouldn't be reactive and cause a re-render - would be nice to not need to call twice and have a dependency array replace the callback...

[verekia]

In React, stale closures mainly happen when the execution is delayed from the render. For instance if you have a setTimeout that triggers something 5 seconds after a click, and you do more state changes before the end of those 5 seconds. But for sync things, there is not problem with my example above, callbacks are re-evaluated at each render, so they are in the execution scope of the current render.

FYI, I just tried react-three-fiber with the same example, and it correctly logs the current state in the handler:

const Counter = () => {
  const [count, setCount] = useState(0)
  const ref = useRef(null)

  const handleClick = () => {
    console.log(count) // Logs 0, 1, 2, 3...
    setCount(count + 1)
  }

  return (
    <mesh
      ref={ref}
      onClick={handleClick}
      ...
    >
  )

I'm not sure how they do it, and I am definitely not experienced enough in either library to have ideas on how to handle this. The way they do it is ~maybe in this file?~

EDIT: ~I think it's here~: https://github.com/pmndrs/react-three-fiber/blob/master/packages/fiber/src/core/renderer.ts#L475

~In this function they do~:

removeChild(parent, instance)
appendChild(parent, newInstance)

[brianzinn]

It's done in the applyProps in renderer.ts - same as done in this project (and other renderers). The handleclick is re-assigned to r3f prop and that difference is detected and re-applied.

I will update the hook to assign the handler being passed in on each React render. I think I just need to add onClick to the useRef dependency array and it will re-assign every React render (since it will be a different instance). I didn't add those hooks to this library, but can see others having same issue. Thanks for reporting and providing the helpful examples.

[SKTT1Ryze]

How about adding an optional argument to useClick like this?

type DependencyList = ReadonlyArray<unknown>;

export function useClick(
  onClick: MeshEventType,
  ownRef?: MutableRefObject<Nullable<Mesh>>,
  deps?: DependencyList
): [MutableRefObject<Nullable<Mesh>>] {
  const createdRef = useRef<Nullable<Mesh>>(null)
  const ref = ownRef ?? createdRef

  useEffect(() => {
    if (ref.current) {
      if (ref.current instanceof AbstractMesh) {
        const mesh = ref.current as Mesh

        if (!mesh.actionManager) {
          mesh.actionManager = new ActionManager(mesh.getScene())
        }

        const action: Nullable<IAction> = mesh.actionManager.registerAction(
          new ExecuteCodeAction(ActionManager.OnPickTrigger, function (ev: any) {
            onClick(ev)
          })
        )
        return () => {
          // unregister on teardown
          mesh.actionManager?.unregisterAction(action!)
        }
      } else {
        console.warn('onClick hook only supports referencing Meshes')
      }
    }
  }, [...(deps || []), ref])
  // todo: if use ref.current as dep,  duplicate register action.

  return [ref]
}

BTW, how can i solve this issue currently?

Thanks.

[brianzinn]

@SKTT1Ryze - does this help? https://epicreact.dev/why-you-shouldnt-put-refs-in-a-dependency-array/

[SKTT1Ryze]

@SKTT1Ryze - does this help? https://epicreact.dev/why-you-shouldnt-put-refs-in-a-dependency-array/

Yes, it can. Thank you so much.

[brianzinn]

This is from 2 years ago - sorry if it's not fixed. I am just cleaning up inactive issues. Kindly re-open if it's not resolved. Cheers.

[verekia]

No problem, do what's best for the library!

[brianzinn]

actually @verekia i see how you flow the refs through - good idea - if you wanted to do a PR, i would accept it.

[verekia]

Sorry, I am not currently using the library or Babylon.js 🙏 I am on React Three Fiber (which could be a good source of inspiration).

[brianzinn]

thanks @verekia ❤️ - an enormous amount of inspiration can be drawn from not only that project, but the entire pmndrs collective 😄 to be honest I don't invest the time this project deserves, but I have so many other non-computer priorities.