Skip to content
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

Use useId instead of requiring consumers to provide an id as a prop #2575

Open
johncowen opened this issue Jan 16, 2025 · 3 comments
Open

Use useId instead of requiring consumers to provide an id as a prop #2575

johncowen opened this issue Jan 16, 2025 · 3 comments

Comments

@johncowen
Copy link
Contributor

Also see #2562 (review)

Ideally if an id is only required internally by a component for aria or other reasons, then a component should instead use Vue's useId to provide that id instead of requiring the consumer of the component to provide one.

This goes for any component that uses id similarly not just KCodeBlock (which the above comment/link refers to)

@Justineo
Copy link
Contributor

I think we can make all ids not required and use useId to provide a default value. But specifying an id could be also helpful in many cases.

@adamdehaven
Copy link
Member

Definitely don’t want to remove support for providing an id from the host, but I’m good with generating a default (this is the case for most components, although KCodeBlock isn’t one of them).

Originally we didn’t do this because of Kuma actually as it caused inconsistency in the snapshot testing that had to be addressed.

My only requirement is that we generate the id in an SSR-compatible way as to prevent hydration errors. (Generating with Vue’s native useId() satisfies this requirement)

@johncowen
Copy link
Contributor Author

johncowen commented Jan 17, 2025

Thanks for the info, providing a default at a minimum sounds fine to me!

But specifying an id could be also helpful in many cases.

Definitely don’t want to remove support for providing an id from the host

Interested in understanding cases where it's helpful to have the consumer provide one (I guess apart from cases where you'd use Vue HTML attrs to add an id instead)

lmk!

Originally we didn’t do this because of Kuma actually as it caused inconsistency in the snapshot testing that had to be addressed.

Not totally aware of the ins-and-outs of this as I think this was when I'd just started working on Kuma, but FYI we got rid of snapshot testing as soon as we could. So whatever this reason is, its not necessary for Kuma anymore.

Plus, I think with the monotonic/lamport-clock-like ++ approach to unique id generation we added in Kuma (and since 3.5 is in Vue via useId) would prevent any issues around this (and SSR). I would guess this approach maintains stability, something a random id generator wouldn't. I suppose using Vue 3.5's useId would also solve whatever this issue was for anyone else who might be using snapshot testing.

My only requirement is that we generate the id in an SSR-compatible way as to prevent hydration errors. (Generating with Vue’s native useId() satisfies this requirement)

Yep absolutely, I made sure to put this in the issue description, agree using Vue's useId a requirement 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants