-
-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Preact X useId #3583
Preact X useId #3583
Conversation
Size Change: +463 B (0%) Total Size: 52.8 kB
ℹ️ View Unchanged
|
Any movement on this? |
@mjgerace I believe Jovi has a PR Open in render-to-string to add the necessary links for this. |
Hi just to let you know I checked out this and the PR on render-to-string (preactjs/preact-render-to-string#226) In my setup this line breaks: Lines 36 to 39 in 186f5da
This is because indexOf is not defined on 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. |
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 😅 |
It breaks during the |
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 ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! 💯
@developit had an alternative that had a better byte-size impact |
Is this okay to merge? |
Any news here? |
2342e42
to
bf5f5f6
Compare
f7de099
to
1202b83
Compare
Let's merge this 👍 |
will there be a new release soon that integrates this ? |
@PodaruDragos I definitely share your excitement about |
@marvinhagemeister my apologies for sounding demanding, yeah I am really waiting for this for a long time. |
Currently to the thought of a prefix tree this would be an alternative to
useId
. This tries to follow our component tree and has amask
variable.Resolves #3373
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 invokeuseId
we will look at thelocalIdCounter
(reset for every component we see) this with the combination of our prefix will ensure that we generate a consistentid
even if we are invoking multipleuseId
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 themask
counter for SSR environments we would bypass this.Notes
DEMO: https://codesandbox.io/s/crazy-platform-omyl5l?file=/src/hooks.js