Skip to content

Commit

Permalink
Generate unique ids within each React island (#6976)
Browse files Browse the repository at this point in the history
  • Loading branch information
SudoCat authored May 4, 2023
1 parent dfb9e42 commit ca329bb
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 14 deletions.
5 changes: 5 additions & 0 deletions .changeset/happy-ears-call.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@astrojs/react': patch
---

Prevent ID collisions in React.useId
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import React from 'react';

export default function () {
const id = React.useId();
return <p className='react-use-id' id={id}>{id}</p>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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!',
Expand All @@ -34,5 +35,7 @@ const someProps = {
<CloneElement />
<WithChildren client:load>test</WithChildren>
<WithChildren client:load children="test" />
<WithId client:idle />
<WithId client:idle />
</body>
</html>
9 changes: 7 additions & 2 deletions packages/astro/test/react-component.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
7 changes: 5 additions & 2 deletions packages/integrations/react/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}
Expand All @@ -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);
});
};
24 changes: 24 additions & 0 deletions packages/integrations/react/context.js
Original file line number Diff line number Diff line change
@@ -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;
}
31 changes: 21 additions & 10 deletions packages/integrations/react/server.js
Original file line number Diff line number Diff line change
@@ -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');
Expand Down Expand Up @@ -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)) {
Expand All @@ -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);
Expand All @@ -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);
});
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit ca329bb

Please sign in to comment.