Skip to content

chore: 🤖 replace react dom server render to static markup by create root#926

Merged
punkbit merged 2 commits intomainfrom
chore/replace-react-dom-server-render-to-static-markup
Mar 12, 2026
Merged

chore: 🤖 replace react dom server render to static markup by create root#926
punkbit merged 2 commits intomainfrom
chore/replace-react-dom-server-render-to-static-markup

Conversation

@punkbit
Copy link
Collaborator

@punkbit punkbit commented Mar 12, 2026

Why?

The following PR removes react-dom/server renderToStaticMarkup from copy grid elements of Grid to favour the recommended createRoot approach suggested by React's official documentation, because react dom's server version is not recommended by react themselves and shouldn't be surfacing publicly in the context of a component library.

According to React renderToStaticMarkup "The renderToStaticMarkup works in the browser, but using it in the client code is not recommended. If you need to render a component to HTML in the browser" and React renderToString "Importing react-dom/server on the client unnecessarily increases your bundle size and should be avoided. If you need to render some component to HTML in the browser, use createRoot and read HTML from the DOM".

How?

  • Adapted the react documented snippted locally

Preview?

Screen.Recording.2026-03-12.at.16.13.03.mov

@changeset-bot
Copy link

changeset-bot bot commented Mar 12, 2026

🦋 Changeset detected

Latest commit: 97d804b

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

This PR includes changesets to release 1 package
Name Type
@clickhouse/click-ui Minor

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
Copy link

github-actions bot commented Mar 12, 2026

Claude finished @punkbit's task in 1m 7s —— View job


PR Review

Good change! Removing react-dom/server from client-side code reduces bundle size and follows React's official guidance. The createRoot + flushSync pattern used here matches exactly what the React docs recommend for rendering to static HTML in the browser.

  • Gather context
  • Review changed files
  • Post findings

🚨 Must Fix

  • Changeset type: The changeset is marked as minor but this is a purely internal refactor with no public API changes. It should be patch to avoid an unwarranted minor version bump.
    Fix this →

💡 Suggestions (non-blocking)

  • Performance (copyGridElements.ts:24): A new React root is created and unmounted for every single cell in the selection. For large copy operations this means potentially hundreds of createRoot/flushSync/unmount cycles, which is heavier than the old synchronous renderToStaticMarkup. This was the trade-off accepted by following React's guidance, but worth documenting or profiling with large datasets.

  • container never attached to the DOM: createRoot on a detached node is fine per React docs (the flushSync ensures synchronous render), but React may log a warning in some versions. This is fine as-is.

@workflow-authentication-public
Copy link
Contributor

📚 Storybook Preview Deployed

✅ Preview URL: https://click-m42xretyr-clickhouse.vercel.app

Built from commit: aedb947a36d2929b8051f3dd35070e6a101727ed

@punkbit punkbit merged commit 1869f03 into main Mar 12, 2026
11 checks passed
@punkbit punkbit deleted the chore/replace-react-dom-server-render-to-static-markup branch March 12, 2026 17:18
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.

2 participants