Fix useId uniqueness with shared parents + DOM nodes in between

[marvinhagemeister]

This PR changes the id generation for useId and fixes an issue where the id wasn't unique anymore when two components were in different HTML nodes, but shared the same component parent.

Instead of storing a _mask property on every component vnode, regardless of whether useId is used or not, we'll only calculate the id when useId is called. When that happens we'll walk up the tree and count the component vnodes up until we reach the root. Then we create a component depth array and increase the component depth index of the current component. Joining all indexes will yield the final ID.

Whilst this PR drops the need for vnode._mask it stores the component depth array on the root vnode as vnode._ids. This behavior is similar to before where ids have the constraint that they are unique per root, not globally unique.

Fixes #3772 .

[coveralls]

Coverage increased (+0.0007%) to 99.531% when pulling 979c4e6 on use-id-dom into 16d570f on master.

[github-actions]

Size Change: -196 B (0%)

Total Size: 52.9 kB

Filename	Size	Change
hooks/dist/hooks.js	1.49 kB	-65 B (4%)
hooks/dist/hooks.module.js	1.52 kB	-66 B (4%)
hooks/dist/hooks.umd.js	1.57 kB	-65 B (4%)

ℹ️ View Unchanged

Filename	Size	Change
compat/dist/compat.js	3.81 kB	0 B
compat/dist/compat.module.js	3.75 kB	0 B
compat/dist/compat.umd.js	3.87 kB	0 B
debug/dist/debug.js	3.01 kB	0 B
debug/dist/debug.module.js	3.01 kB	0 B
debug/dist/debug.umd.js	3.09 kB	0 B
devtools/dist/devtools.js	231 B	0 B
devtools/dist/devtools.module.js	240 B	0 B
devtools/dist/devtools.umd.js	314 B	0 B
dist/preact.js	4.06 kB	0 B
dist/preact.min.js	4.08 kB	0 B
dist/preact.min.module.js	4.08 kB	0 B
dist/preact.min.umd.js	4.11 kB	0 B
dist/preact.module.js	4.07 kB	0 B
dist/preact.umd.js	4.12 kB	0 B
jsx-runtime/dist/jsxRuntime.js	358 B	0 B
jsx-runtime/dist/jsxRuntime.module.js	324 B	0 B
jsx-runtime/dist/jsxRuntime.umd.js	439 B	0 B
test-utils/dist/testUtils.js	442 B	0 B
test-utils/dist/testUtils.module.js	444 B	0 B
test-utils/dist/testUtils.umd.js	526 B	0 B

_{compressed-size-action}

[SvizelPritula]

Hi,
what is the point of using an array with several counters insted of a single counter? Since the assigned ids are already dependant on the mount order (which is probably unavoidable), a single "global" (stored in the root component) counter would accomplish the same thing.

[JoviDeCroock]

A single counter would not credit async boundaries that could be handled differently during SSR vs CS hydration, the only deterministic similarity should be the shape of the vdom tree after all boundaries resolve

[SvizelPritula]

Right, but the current implementation also depends on mount order:

import { useId, useState, useEffect } from 'preact/hooks';

function Id() {
  let id = useId();
  return <div id={id}>{id}</div>;
}

function Eventually({ children }) {
  let [resolved, setResolved] = useState(false);
  useEffect(() => {
    setTimeout(() => setResolved(true), Math.random() * 1000);
  }, []);

  return <div>
    {resolved && children}
  </div>;
}

function Repeat({ children, count }) {
  let array = Array.from(Array(count).keys());
  return array.map(v => <div key={v}>{children}</div>);
}

export default function App() {
  return <div>
    <Repeat count={10}>
      <Eventually>
        <Id />
      </Eventually>
    </Repeat>
  </div>;
}

This code assigns different ids on every render.

[marvinhagemeister]

I think you're right, we could optimize this further. The main need is to differentiate between counters in async boundaries (=Suspense or Islands) as they could be streamed to the client out of order. It's not ready yet, but I'm working on a streaming variant of preact-render-to-string. Thinking about this again maybe all that's needed is to just add a prefix for each async boundary and the rest could be a counter scoped to the the current subtree.

[SvizelPritula]

If you didn't care about the id length (which, sadly, impacts the total size of the prerendered html), you could use this:

export function useId() {
	const state = getHookState(currentIndex++, 11);

	if (!state._value) {
		let node = currentComponent._vnode;
		console.log(node);
		let parent = node._parent;

		let path = [currentIndex];

		while (parent !== null) {
			path.push(node.key ?? parent._children.indexOf(node));

			node = parent;
			parent = node._parent;
		}

		state._value = 'P' + path.join('-');
	}

	return state._value;
}

This would make each id a path from the root element to the calling component. (In reverse order, but that hardly matters.)

The main problem is that this relies on keys being set correctly.

[SvizelPritula]

The id concatenation has some issues. (See comment)