Any reason to not reuse the same logic from #60879?

The situation is very similar, but not entirely identical. Here, when adding refs to the map, we are merely registering them, without firing listeners. It's only when the ref callback actually sets the ref.current when the listeners are fired.

Maybe it's possible to merge the two implementations, but I'd rather do it in steps.

I don't remember why there were multiple refs 😬

There are multiple refs if you call useBlockRefProvider( clientId ) twice for the same clientId:

const refA = useBlockRefProvider( clientId );
return <div ref={ refA } />;

// somewhere else, with same clientId but different element
const refB = useBlockRefProvider( clientId );
return <div ref={ refB } />;

The it registers two refs for the same clientId, and useBlockRef/useBlockElement will return the one that's non-null.

I also don't know if this ever happens 🙂

I wonder if we should just error if we try to call it twice with the same clientId

Yes, ignoring the second call and printing a warning would certainly simplify the code. Then we can store just one element instead of a Set.

Can't we reuse the same callback for the layout effect as well? The benefit of useRefEffect is that you can pass a cleanup function just like useLayoutEffect, so I think we could add/remove to/from the map as well in addition to callListeners?

What do you mean by reuse?

I looked more closely at the lifecycle of this hook and some things are suspicious:

1. On mount, the DOM element is created, and the ref callback called. This callback sets the ref.current and calls listeners. But at this moment, the effect that adds the ref to the map hasn't yet run, so the listeners don't see the element yet! useBlockElement( clientId ) hooks that started observing clientId before it was mounted will get null as return value. That looks bad! In trunk this works ok because the element is set directly with local setElement state setter, the map is not used. Only this branch is broken.
2. Now the effect that calls addToMap runs. But there's no calling of listeners. Observing useBlockElement hooks won't be notified.
3. What if, during the lifecycle of the component, the reffed element changes? Consider code like this:

   const ref = useBlockRefProvider( clientId );
   return variant ? <b ref={ ref } /> : <i ref={ ref }>;

   When the element changes like this, the ref callback will be first called with null. But our callback ignores null values, so it will do nothing. Then the ref callback is called with the new element. That updates the ref.current and notifies the listeners, and they will see the new element. That's great.
4. When the component is unmounting, first the ref callback is called with null. Nothing happens, listeners are not notified.
5. Then the cleanup callback of the effect is called and the ref is removed from the map.
6. If a component that calls useBlockElement is rerendered for a random reason, then i) in trunk it will return the element that no longer exists, because it's stored in the hook's local state; ii) in this branch it will return null because the getValue function for uSES won't find the map record. In both branches the hook won't be notified about the element change, we need to wait for a random rerender triggered by something else.

To conclude, this PR still needs more testing and more work.

That's what I meant here: addToMap should move to the ref callback, right before callListeners? The ref callback will also be called when then a ref is attached to a newly mounted component (your snipper), so the ref will be re-added and listeners called again.

addToMap should move to the ref callback

The disadvantage of this is that when the element changes, the listeners will get two notifications: one about null element and one about the new element. While ideally they should be notified only about the new element. But it's an edge case anyway, and it's not harmful, just slightly inefficient.