Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate unique ids within each React island #6976

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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