Generate unique ids within each React island

[SudoCat]

Resolves #6849

React's useId hook generates unique IDs by incrementing down the component tree. When multiple react roots are on a single page, these IDs can collide. React provides an option, identifierPrefix to prefix ids per root.

Using preact adapters context solution, increment an ID per component rendered on a request, preventing collisions.

Changes

• Ensures each react island can generate unique ids without collisions, improve accessibility
• follows the context pattern used by preact-adapter to increment id per component
• incremental id is added to astro-island element attributes, named prefix
• id is passed to react server rendering methods directly as identifierPrefix option
• id is read from attributes client side, and passed as options to createRoot/hydrateRoot as indetifierPrefix

Testing

• tested via local linking to validate results
• test case added to react test suite to validate that two ids generated in separate islands are unique.

Docs

Unsure if documentation is needed; shouldn't change anything for users - something that didn't work properly will now work as intended.

/cc @withastro/maintainers-docs for feedback!

[changeset-bot]

🦋 Changeset detected

Latest commit: af4ed8c

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[SudoCat]

@ematipico Most of the comments you've raised related to the context implementation, so I wanted to tackle them in one thread.

I literally copy and pasted the context file from the preact adapter.
https://github.com/withastro/astro/blob/main/packages/integrations/preact/src/context.ts

Of course, the original was in typescript, so I removed the typings, which likely added a lot more of the context you're requesting here. (Unfortunatley, the react adapter only compiles the index file, and I wanted to keep the changes to a minimum to help get this issue resolved promptly.)

I'm happy to make changes, my thoughts:

• let -> const (yes, very sensible, not sure why it's let in the preact one)
• result -> rendererContextResult - much more verbose. This information was provided by types in the preact adapter, so using a more verbose name here seems best.
• r -> I'll create a constant named ID_PREFIX - I don't see much issue with the value remaining as r though, as its short-and-sweet, and keeps the ID's generated by react nice and short. React's useId also prefixes the values with a capital r, resulting in :r0R0 which seems fairly concise. Happy to change if you have a better suggestion though - it's only a minor detail. What's the naming convention for constants; SCREAMING_SNAKES or something else?
• c - i have no idea what to name this. currentIndex or current maybe?
• avoid exposing getContext - you might be right. This was an effect of copying the implementation from preact, whereas their implementation does actually use the getContext method standalone for certain cases. Happy to make this change if you still feel it's the right choice. Personally, were I to write it myself, I think I'd add an incrementId() method on the context object, and do something more like getContext(result).incrementId() as this feels more transparent to me.

I think you're right on most points, I just tried to keep the code as close to the original, figuring if that had already been accepted, it must be the preferred standard 😅

[ematipico]

Sorry @SudoCat for my pedantic comments! I looked at the PR with almost no context and knowledge of the existing code 😅

[SudoCat]

No worries at all! Your pedantry aligns well with my own. I'm very grateful for a prompt review, anything to help get this accessibility issue fixed and shipped 🎉