preact icon indicating copy to clipboard operation
preact copied to clipboard

Preact X useId alternative

Open JoviDeCroock opened this issue 2 years ago • 2 comments

Currently to the thought of a prefix tree this would be an alternative to useId. This tries to follow our component tree and has a mask variable.

Mount

Every time we encounter a component a long the way we will set vnode._mask to the global counter we are following, this means that every node that can generate an id will have a unique mask. When we invoke useId we will look at the localIdCounter (reset for every component we see) this with the combination of our prefix will ensure that we generate a consistent id even if we are invoking multiple useId calls.

Mounting a new subtree

When we see a new subtree we will look at the parentMask and count from there, this however means that we could cause conflicts, ideally we would need to keep incrementing mask. If we can find a way to reset the mask counter for SSR environments we would bypass this.

Notes

  • We need to add _children and _parent to render to string
  • We can optimize subsequent client-side renders by using a counter that increments after hydration

Going to wait for an OK before I get started on the RTS work

JoviDeCroock avatar Jun 23 '22 15:06 JoviDeCroock

Size Change: +463 B (0%)

Total Size: 52.8 kB

Filename Size Change
compat/dist/compat.js 3.78 kB +7 B (0%)
compat/dist/compat.module.js 3.72 kB +11 B (0%)
compat/dist/compat.umd.js 3.85 kB +5 B (0%)
hooks/dist/hooks.js 1.55 kB +151 B (9%) 🔍
hooks/dist/hooks.module.js 1.57 kB +141 B (8%) 🔍
hooks/dist/hooks.umd.js 1.63 kB +148 B (9%) 🔍
ℹ️ View Unchanged
Filename Size Change
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 315 B 0 B
dist/preact.js 4.01 kB 0 B
dist/preact.min.js 4.04 kB 0 B
dist/preact.min.module.js 4.04 kB 0 B
dist/preact.min.umd.js 4.07 kB 0 B
dist/preact.module.js 4.03 kB 0 B
dist/preact.umd.js 4.08 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

github-actions[bot] avatar Jun 23 '22 15:06 github-actions[bot]

Coverage Status

Coverage increased (+0.005%) to 99.527% when pulling 1aad6d8066c5c9c0bcab5d3c8c20c1bf2eaf8cdc on use-id-alternative into 1427d58fb43f8b42013b340cbff0cfc72ca52d0e on master.

coveralls avatar Jun 23 '22 15:06 coveralls

Any movement on this?

mjgerace avatar Aug 16 '22 18:08 mjgerace

@mjgerace I believe Jovi has a PR Open in render-to-string to add the necessary links for this.

developit avatar Aug 17 '22 12:08 developit

Hi just to let you know I checked out this and the PR on render-to-string (https://github.com/preactjs/preact-render-to-string/pull/226)

In my setup this line breaks: https://github.com/preactjs/preact/blob/186f5da5c8965fccd2760bacf76d7d75e8769d53/hooks/src/index.js#L36-L39

This is because indexOf is not defined on vnode._parent._children, I investigated this and it seems like _children is a rendered preact object rather than a list. In my case I fixed it by just making the line:

const position = vnode._parent &&
    vnode._parent._children &&
    vnode._parent._children !== vnode &&
    vnode._parent._children.indexOf

I realise this is quite hacky (and probably doesn't do what I actually want it to) but at least it doesn't crash anymore. I have basically no knowledge of this project and just needed stuff to work.

itsbjoern avatar Aug 17 '22 20:08 itsbjoern

You mean it breaks on the client or during SSR, if it breaks on the client that might be expected I haven't gone through all cases yet.

As mentioned this is in no way a done thing 😅

JoviDeCroock avatar Aug 17 '22 20:08 JoviDeCroock

It breaks during the render call of render-to-string.

itsbjoern avatar Aug 17 '22 20:08 itsbjoern

The more I think about it, given the scuffed setup I'm using it is very possible that the fault is actually somewhere on my end ...

itsbjoern avatar Aug 17 '22 20:08 itsbjoern

@developit had an alternative that had a better byte-size impact

JoviDeCroock avatar Aug 23 '22 20:08 JoviDeCroock

Is this okay to merge?

mjgerace avatar Aug 24 '22 22:08 mjgerace

Any news here?

Wallacy avatar Aug 30 '22 01:08 Wallacy

Let's merge this 👍

marvinhagemeister avatar Sep 02 '22 19:09 marvinhagemeister

will there be a new release soon that integrates this ?

PodaruDragos avatar Sep 04 '22 16:09 PodaruDragos

@PodaruDragos I definitely share your excitement about useId! We merged this PR just two days ago and it was the weekend. We maintainers need some time to recharge and relax too :)

marvinhagemeister avatar Sep 04 '22 20:09 marvinhagemeister

@marvinhagemeister my apologies for sounding demanding, yeah I am really waiting for this for a long time. Thanks for this feature and thanks for all the good work you guys are doing on this project.

PodaruDragos avatar Sep 05 '22 06:09 PodaruDragos

When will the new version be released?

Aloento avatar Sep 10 '22 09:09 Aloento