Skip to content

feat: update useGeneratedHtmlId to use React.useId when available#7095

Merged
tkajtoch merged 7 commits intoelastic:mainfrom
tkajtoch:feat/ssr-html-id-generator
Aug 21, 2023
Merged

feat: update useGeneratedHtmlId to use React.useId when available#7095
tkajtoch merged 7 commits intoelastic:mainfrom
tkajtoch:feat/ssr-html-id-generator

Conversation

@tkajtoch
Copy link
Copy Markdown
Member

@tkajtoch tkajtoch commented Aug 17, 2023

Summary

This PR resolves #7078 by updating useGeneratedHtmlId utility function to use React.useId when it's available (which means when running React 18).

It's the first step in making useGeneratedHtmlId and its usages SSR-compatible. We must do #7094 to make useGeneratedHtmlId SSR-safe across all supported React versions.

QA

  • Make sure all tests are passing
  • Verify that <EuiSwitch> and <EuiDataGrid> generated DOM id attributes reference the same objects as before. The id values will be different (instead of UUIDs there's gonna be strings like :r1: generated by React.useId)

General checklist

  • Checked in mobile
  • Checked in Chrome, Safari, Edge, and Firefox
  • Checked Code Sandbox works for any docs examples
  • Checked for breaking changes and labeled appropriately
  • Checked for accessibility including keyboard-only and screenreader modes
  • A changelog entry exists and is marked appropriately

@tkajtoch tkajtoch self-assigned this Aug 17, 2023
@kibanamachine
Copy link
Copy Markdown

Preview documentation changes for this PR: https://eui.elastic.co/pr_7095/

@tkajtoch tkajtoch force-pushed the feat/ssr-html-id-generator branch from 75f875e to 7c76542 Compare August 18, 2023 08:31
@tkajtoch tkajtoch marked this pull request as ready for review August 18, 2023 08:44
@kibanamachine
Copy link
Copy Markdown

Preview documentation changes for this PR: https://eui.elastic.co/pr_7095/

1 similar comment
@kibanamachine
Copy link
Copy Markdown

Preview documentation changes for this PR: https://eui.elastic.co/pr_7095/

Comment on lines +163 to +164
// pageInnerId may contain colons that are parsed as pseudo-elements if not escaped
parent: `#${CSS.escape(pageInnerId)}`,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

TIL there's a global CSS var, and TIL CSS.escape 🤯

@tkajtoch just to confirm - does this global exist on SSR as well as in browsers?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

it's available in all modern browsers (caniuse) and Next.js client components. Other SSR environments may or may not have it, but usually they're using JSDOM polyfills for this and other very common properties like HTMLElement or various observers.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sounds good to me, Next.js will likely be our biggest SSR framework in terms of usage in any case.

Not a change request, but if this comes up in the future from a consumer we could also consider a basic regex like .replace(/:/g, '\\:') instead if colons are the only issue here - I would consider that a lesser lift on our part than consumers needing a polyfill.

@tkajtoch tkajtoch force-pushed the feat/ssr-html-id-generator branch from 983da16 to 7e59b17 Compare August 21, 2023 18:48
@kibanamachine
Copy link
Copy Markdown

Preview documentation changes for this PR: https://eui.elastic.co/pr_7095/

Copy link
Copy Markdown
Contributor

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

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

Tests look fantastic - thanks so much! I now have a much clearer idea of what useId does/looks like

@tkajtoch tkajtoch enabled auto-merge (squash) August 21, 2023 19:46
@kibanamachine
Copy link
Copy Markdown

Preview documentation changes for this PR: https://eui.elastic.co/pr_7095/

@elasticmachine
Copy link
Copy Markdown
Collaborator

💚 Build Succeeded

History

cc @tkajtoch

@tkajtoch tkajtoch merged commit 5455e05 into elastic:main Aug 21, 2023
breehall added a commit to elastic/kibana that referenced this pull request Aug 23, 2023
`87.1.0` ➡️ `87.2.0`

## [`87.2.0`](https://github.com/elastic/eui/tree/v87.2.0)

- `EuiResizableButton` is now available as a generic top-level export
([#7087](elastic/eui#7087))
- Added new `alignIndicator` prop to `EuiResizableButton`. Defaults to
`center`, and can now additionally be configured to `start` and `end`
([#7087](elastic/eui#7087))
- Updated `useGeneratedHtmlId` hook to use `React.useId` as the source
of unique identifiers when available
([#7095](elastic/eui#7095))

**CSS-in-JS conversions**

- Converted `EuiResizableButton` to Emotion; Removed
`$euiResizableButtonTransitionSpeed` and `$euiResizableButtonSize`
([#7081](elastic/eui#7081))
- Converted `EuiResizableCollapseButton` to Emotion
([#7091](elastic/eui#7091))

---------

Co-authored-by: Cee Chen <constance.chen@elastic.co>
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

Successfully merging this pull request may close these issues.

Update useGeneratedHtmlId to use SSR-compatible useId hook when available

4 participants