Unexpected null ref

[jakearchibald]

• Check if updating to the latest Preact version resolves the issue

Describe the bug

A ref is unexpectedly null when using a particular JSX structure.

To Reproduce

Demo.

View the console. I see:

{phase: 1, oneRef: null, twoRef: div, threeRef: div}
{phase: 2, oneRef: div, twoRef: null, threeRef: div}

Expected behaviour

{phase: 1, oneRef: null, twoRef: div, threeRef: div}
{phase: 2, oneRef: div, twoRef: div, threeRef: div}

It's expected that oneRef is null in phase 1 (the element isn't rendered), but twoRef shouldn't be null in phase 2, as the element is rendered.

The bug seems to depend on a few things:

If <div class="wrapper"> is switched to a fragment, the bug still happens. However, if <div class="wrapper"> is removed, the bug does not happen.

If <div class="outer"> is switched to a fragment, the bug does not happen.

[developit]

Ah, yeah. This is a bug in the ref "unset" code. When I switch to function refs, I can see the following ref updates:

// phase 1
assigning twoRef <div>​Element two​</div>​
assigning threeRef <div>​Element three​</div>​

// phase 2
assigning twoRef <div>​Element two​</div>​
assigning threeRef <div>​Element three​</div>​
assigning twoRef null    // <--- this the bug
assigning oneRef <div>​Element one​</div>​

[jakearchibald]

I guess the unsetting code needs to check that it's unsetting the value it set.

[JoviDeCroock]

Hmm, I have investigated this issue before while working on v11. Basically the issue stems from the parent indentation being introduced. We recurse down the tree initially and see the following

<div> <-- depth 1
  <div ref={twoRef} <-- depth 2
  <div ref={threeRef} <-- depth 2

Notice the depth annotation as this will play a role here, when we go into the second phase we see the following

<div> <-- depth 1
  <div ref={oneRef} <-- depth 2
  <div <-- depth 2
    <div ref={twoRef} <-- depth 3
    <div ref={threeRef} <-- depth 3

So while we go through the tree we'll end up at depth 3 first to start performing operations, we see that twoRef and threeRef are "new" as there was nothing on depth 3 before so we create the elements and we assign the refs. We continue on to depth 2 where we notice a difference as we now miss twoRef and have a new oneRef, we'll create the oneRef VNode and unmount the twoRef VNode, this unmount leads us to call it with null to properly clean up the ref.

I Acknowledge that this is a very peculiar scenario and we might be able to circumvent this by first going through the tree and registering stuff for unmount and queueing the new refs to apply after.

[developit]

Yeah I think this basically requires doing all ref updates in commit. To do that we need to expand the definition of what we store in the commit queue to include different functions (basically opcodes).

[JoviDeCroock]

The concern I have are layout and normal effects we need to guarantee refs being present 😅 I think storing it in the commit queue will indeed fix this

[jakearchibald]

Here's a little workaround in case people need it:

import { useRef } from "preact/hooks";

const modifiedThisPass = new WeakSet();
const shittyProxies = new WeakMap();

export function shittyElRef() {
  const ref = useRef(null);

  if (!shittyProxies.has(ref)) {
    shittyProxies.set(ref, {
      get current() {
        return ref.current;
      },
      set current(val) {
        if (modifiedThisPass.has(ref)) {
          // If this has already been set this pass,
          // ignore null values
          if (val === null) return;
        } else {
          modifiedThisPass.add(ref);
          queueMicrotask(() => modifiedThisPass.delete(ref));
        }

        ref.current = val;
      },
    });
  }

  return shittyProxies.get(ref);
}

It tries to ensure that refs can't be set from a value to null in the same pass. Demo.

[JoviDeCroock]

@jakearchibald this should be fixed in 10.16.0, thank you so much again for the reproduction! It helped a ton in identifying this!

[jakearchibald]

Thanks for fixing it so quickly!