Skip to content

fix: prevent null prefixed results from React.useId when using client:only directive - #8075

Merged
natemoo-re merged 2 commits into
withastro:mainfrom
SudoCat:feat/react-clientside-identifier-prefix
Aug 14, 2023
Merged

natemoo-re merged 2 commits into
withastro:mainfrom
SudoCat:feat/react-clientside-identifier-prefix

Conversation

@SudoCat

@SudoCat SudoCat commented Aug 14, 2023

Copy link
Copy Markdown
Contributor

There was a low-impact bug with the unique id generation fix, causing client:only ids to be generated with null, e.g. :nullr0:

The ids generated are actually still unique, and avoid clashes with server-side, but it was flagged by a team member as a possible bug, so I felt an upstream fix was wise.

Changes

  • removes the identifierPrefix from client:only components, to prevent client:only useId calls from being prefixed with null

Testing

  • quick test in browser via linking
  • additional e2e test case added to validate clientside results are unique, and do not clash with static or hydrated server-rendered results.

Docs

No docs required, no change to user behaviour.

@changeset-bot

changeset-bot Bot commented Aug 14, 2023 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 40c1fa5

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

@github-actions github-actions Bot added pkg: react Related to React (scope) pkg: integration Related to any renderer integration (scope) pkg: astro Related to the core `astro` package (scope) labels Aug 14, 2023
@SudoCat
SudoCat force-pushed the feat/react-clientside-identifier-prefix branch from 30806ca to b299846 Compare August 14, 2023 15:24
@SudoCat
SudoCat requested a review from a team as a code owner August 14, 2023 15:24
@SudoCat
SudoCat force-pushed the feat/react-clientside-identifier-prefix branch from b299846 to c6e8429 Compare August 14, 2023 15:25
This was causing React components rendered with client:only
to be prefixed with null. While not technically causing any
issues, it is unintended and could be considered a bug.
@SudoCat
SudoCat force-pushed the feat/react-clientside-identifier-prefix branch from c6e8429 to 411b612 Compare August 14, 2023 15:26

@natemoo-re natemoo-re left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix, really appreciate the PR!

@natemoo-re
natemoo-re removed the request for review from a team August 14, 2023 16:00
@natemoo-re
natemoo-re merged commit da517d4 into withastro:main Aug 14, 2023
This was referenced Aug 14, 2023
@SudoCat
SudoCat deleted the feat/react-clientside-identifier-prefix branch August 18, 2023 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg: astro Related to the core `astro` package (scope) pkg: integration Related to any renderer integration (scope) pkg: react Related to React (scope)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants