From ca329bbcae7a6075af4f428f6f64466e9d152c8f Mon Sep 17 00:00:00 2001 From: Robin Neal Date: Thu, 4 May 2023 15:23:00 +0100 Subject: [PATCH] Generate unique ids within each React island (#6976) --- .changeset/happy-ears-call.md | 5 +++ .../react-component/src/components/WithId.jsx | 6 ++++ .../react-component/src/pages/index.astro | 3 ++ packages/astro/test/react-component.test.js | 9 ++++-- packages/integrations/react/client.js | 7 +++-- packages/integrations/react/context.js | 24 ++++++++++++++ packages/integrations/react/server.js | 31 +++++++++++++------ 7 files changed, 71 insertions(+), 14 deletions(-) create mode 100644 .changeset/happy-ears-call.md create mode 100644 packages/astro/test/fixtures/react-component/src/components/WithId.jsx create mode 100644 packages/integrations/react/context.js diff --git a/.changeset/happy-ears-call.md b/.changeset/happy-ears-call.md new file mode 100644 index 000000000000..42171e51b4e5 --- /dev/null +++ b/.changeset/happy-ears-call.md @@ -0,0 +1,5 @@ +--- +'@astrojs/react': patch +--- + +Prevent ID collisions in React.useId diff --git a/packages/astro/test/fixtures/react-component/src/components/WithId.jsx b/packages/astro/test/fixtures/react-component/src/components/WithId.jsx new file mode 100644 index 000000000000..0abe91c72749 --- /dev/null +++ b/packages/astro/test/fixtures/react-component/src/components/WithId.jsx @@ -0,0 +1,6 @@ +import React from 'react'; + +export default function () { + const id = React.useId(); + return

{id}

; +} diff --git a/packages/astro/test/fixtures/react-component/src/pages/index.astro b/packages/astro/test/fixtures/react-component/src/pages/index.astro index abd3d4299055..3afd8233f2d7 100644 --- a/packages/astro/test/fixtures/react-component/src/pages/index.astro +++ b/packages/astro/test/fixtures/react-component/src/pages/index.astro @@ -8,6 +8,7 @@ import Pure from '../components/Pure.jsx'; import TypeScriptComponent from '../components/TypeScriptComponent'; import CloneElement from '../components/CloneElement'; import WithChildren from '../components/WithChildren'; +import WithId from '../components/WithId'; const someProps = { text: 'Hello world!', @@ -34,5 +35,7 @@ const someProps = { test + + diff --git a/packages/astro/test/react-component.test.js b/packages/astro/test/react-component.test.js index 7205b034289c..3565342c22a4 100644 --- a/packages/astro/test/react-component.test.js +++ b/packages/astro/test/react-component.test.js @@ -42,16 +42,21 @@ describe('React Components', () => { expect($('#pure')).to.have.lengthOf(1); // test 8: Check number of islands - expect($('astro-island[uid]')).to.have.lengthOf(7); + expect($('astro-island[uid]')).to.have.lengthOf(9); // test 9: Check island deduplication const uniqueRootUIDs = new Set($('astro-island').map((i, el) => $(el).attr('uid'))); - expect(uniqueRootUIDs.size).to.equal(6); + expect(uniqueRootUIDs.size).to.equal(8); // test 10: Should properly render children passed as props const islandsWithChildren = $('.with-children'); expect(islandsWithChildren).to.have.lengthOf(2); expect($(islandsWithChildren[0]).html()).to.equal($(islandsWithChildren[1]).html()); + + // test 11: Should generate unique React.useId per island + const islandsWithId = $('.react-use-id'); + expect(islandsWithId).to.have.lengthOf(2); + expect($(islandsWithId[0]).attr('id')).to.not.equal($(islandsWithId[1]).attr('id')) }); it('Can load Vue', async () => { diff --git a/packages/integrations/react/client.js b/packages/integrations/react/client.js index 3807ab41069f..366d499e3ad7 100644 --- a/packages/integrations/react/client.js +++ b/packages/integrations/react/client.js @@ -13,6 +13,9 @@ function isAlreadyHydrated(element) { export default (element) => (Component, props, { default: children, ...slotted }, { client }) => { if (!element.hasAttribute('ssr')) return; + const renderOptions = { + identifierPrefix: element.getAttribute('prefix') + } for (const [key, value] of Object.entries(slotted)) { props[key] = createElement(StaticHtml, { value, name: key }); } @@ -28,10 +31,10 @@ export default (element) => } if (client === 'only') { return startTransition(() => { - createRoot(element).render(componentEl); + createRoot(element, renderOptions).render(componentEl); }); } return startTransition(() => { - hydrateRoot(element, componentEl); + hydrateRoot(element, componentEl, renderOptions); }); }; diff --git a/packages/integrations/react/context.js b/packages/integrations/react/context.js new file mode 100644 index 000000000000..5d9b1d7b1a15 --- /dev/null +++ b/packages/integrations/react/context.js @@ -0,0 +1,24 @@ +const contexts = new WeakMap(); + +const ID_PREFIX = 'r'; + +function getContext(rendererContextResult) { + if (contexts.has(rendererContextResult)) { + return contexts.get(rendererContextResult); + } + const ctx = { + currentIndex: 0, + get id() { + return ID_PREFIX + this.currentIndex.toString(); + }, + }; + contexts.set(rendererContextResult, ctx); + return ctx; +} + +export function incrementId(rendererContextResult) { + const ctx = getContext(rendererContextResult) + const id = ctx.id; + ctx.currentIndex++; + return id; +} diff --git a/packages/integrations/react/server.js b/packages/integrations/react/server.js index 01a135a9ba4c..0d85984f90dd 100644 --- a/packages/integrations/react/server.js +++ b/packages/integrations/react/server.js @@ -1,6 +1,7 @@ import React from 'react'; import ReactDOM from 'react-dom/server'; import StaticHtml from './static-html.js'; +import { incrementId } from './context.js'; const slotName = (str) => str.trim().replace(/[-_]([a-z])/g, (_, w) => w.toUpperCase()); const reactTypeof = Symbol.for('react.element'); @@ -58,6 +59,12 @@ async function getNodeWritable() { } async function renderToStaticMarkup(Component, props, { default: children, ...slotted }, metadata) { + let prefix; + if (this && this.result) { + prefix = incrementId(this.result) + } + const attrs = { prefix }; + delete props['class']; const slots = {}; for (const [key, value] of Object.entries(slotted)) { @@ -74,29 +81,33 @@ async function renderToStaticMarkup(Component, props, { default: children, ...sl newProps.children = React.createElement(StaticHtml, { value: newChildren }); } const vnode = React.createElement(Component, newProps); + const renderOptions = { + identifierPrefix: prefix + } let html; if (metadata && metadata.hydrate) { if ('renderToReadableStream' in ReactDOM) { - html = await renderToReadableStreamAsync(vnode); + html = await renderToReadableStreamAsync(vnode, renderOptions); } else { - html = await renderToPipeableStreamAsync(vnode); + html = await renderToPipeableStreamAsync(vnode, renderOptions); } } else { if ('renderToReadableStream' in ReactDOM) { - html = await renderToReadableStreamAsync(vnode); + html = await renderToReadableStreamAsync(vnode, renderOptions); } else { - html = await renderToStaticNodeStreamAsync(vnode); + html = await renderToStaticNodeStreamAsync(vnode, renderOptions); } } - return { html }; + return { html, attrs }; } -async function renderToPipeableStreamAsync(vnode) { +async function renderToPipeableStreamAsync(vnode, options) { const Writable = await getNodeWritable(); let html = ''; return new Promise((resolve, reject) => { let error = undefined; let stream = ReactDOM.renderToPipeableStream(vnode, { + ...options, onError(err) { error = err; reject(error); @@ -118,11 +129,11 @@ async function renderToPipeableStreamAsync(vnode) { }); } -async function renderToStaticNodeStreamAsync(vnode) { +async function renderToStaticNodeStreamAsync(vnode, options) { const Writable = await getNodeWritable(); let html = ''; return new Promise((resolve, reject) => { - let stream = ReactDOM.renderToStaticNodeStream(vnode); + let stream = ReactDOM.renderToStaticNodeStream(vnode, options); stream.on('error', (err) => { reject(err); }); @@ -164,8 +175,8 @@ async function readResult(stream) { } } -async function renderToReadableStreamAsync(vnode) { - return await readResult(await ReactDOM.renderToReadableStream(vnode)); +async function renderToReadableStreamAsync(vnode, options) { + return await readResult(await ReactDOM.renderToReadableStream(vnode, options)); } export default {