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

React useId cannot generate unique identifiers for islands #6849

Closed
1 task done
SudoCat opened this issue Apr 14, 2023 · 2 comments · Fixed by #6976
Closed
1 task done

React useId cannot generate unique identifiers for islands #6849

SudoCat opened this issue Apr 14, 2023 · 2 comments · Fixed by #6976
Labels
- P3: minor bug An edge case that only affects very specific usage (priority)

Comments

@SudoCat
Copy link
Contributor

SudoCat commented Apr 14, 2023

What version of astro are you using?

2.2.3

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

pnpm

What operating system are you using?

Mac

What browser are you using?

Firefox

Describe the Bug

When rendering React components in separate islands on one page, React's useId hook generates duplicate IDs.

The react useId hook generates incrementing IDs based on the react component tree. As islands have separate roots, they will generate their IDs separately, resulting in collisions.

Many accessibility-focused React UI libraries make use of useId to generate aria attributes. This functionality is currently not compatible with Astro.

React does provide a configuration option, identifierPrefix, which can be passed to createRoot, hydrateRoot, renderToString and similar functions.

If the react renderer adapter passed a unique identifierPrefix when rendering each island, this issue could be resolved.


I attempted adding this in to the react adapter, but I wasn't sure on the best method to generate a consistent unique ID per component during server rendering which could be accessible to client rendering.

At first I considered using the UID attribute on the astro-island wrapping element, unfortunately that isn't available at server render time, as it's generated from hashing the server renderinng response. This value also isn't guaranteed to be unique - if the same component is rendered in two places on a page with the same props, the generated UID is identical.

I'm not sure what the best solution would be, but I'd be more than happy to write up a PR if anyone has some better thoughts on how to achieve this!

Link to Minimal Reproducible Example

https://stackblitz.com/edit/withastro-astro-lriyir?file=src/pages/index.astro

Participation

  • I am willing to submit a pull request for this issue.
@matthewp
Copy link
Contributor

Thanks @SudoCat We do something similar for Preact here: https://github.com/withastro/astro/blob/main/packages/integrations/preact/src/signals.ts#L43

I think the same method could work here. Use this.result which is a unique object for each render, and increment a counter. Then You can pass data attributes to use in the client.

@matthewp matthewp added the - P3: minor bug An edge case that only affects very specific usage (priority) label Apr 28, 2023
@SudoCat
Copy link
Contributor Author

SudoCat commented May 2, 2023

Excellent, this seems like a more practical solution with fewer issues than serializing and hashing the props.

I'll work on porting the preact context solution over to the react adapter, and shall raise a PR if I get everything working. Thanks for pointing me in the right direction!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P3: minor bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants