Skip to content

Commit

Permalink
[Fix] properly track useId use in StrictMode in development (#25713)
Browse files Browse the repository at this point in the history
In `<StrictMode>` in dev hooks are run twice on each render.

For `useId` the re-render pass uses the `updateId` implementation rather
than `mountId`. In the update path we don't increment the local id
counter. This causes the render to look like no id was used which
changes the tree context and leads to a different set of IDs being
generated for subsequent calls to `useId` in the subtree.

This was discovered here: vercel/next.js#43033

It was causing a hydration error because the ID generation no longer
matched between server and client. When strict mode is off this does not
happen because the hooks are only run once during hydration and it
properly sees that the component did generate an ID.

The fix is to not reset the localIdCounter in `renderWithHooksAgain`. It
gets reset anyway once the `renderWithHooks` is complete and since we do
not re-mount the ID in the `...Again` pass we should retain the state
from the initial pass.
  • Loading branch information
gnoff authored Nov 28, 2022
1 parent 8a23def commit 15557fa
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 2 deletions.
64 changes: 64 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMUseId-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -633,4 +633,68 @@ describe('useId', () => {
</div>
`);
});

// https://github.com/vercel/next.js/issues/43033
// re-rendering in strict mode caused the localIdCounter to be reset but it the rerender hook does not
// increment it again. This only shows up as a problem for subsequent useId's because it affects child
// and sibling counters not the initial one
it('does not forget it mounted an id when re-rendering in dev', async () => {
function Parent() {
const id = useId();
return (
<div>
{id} <Child />
</div>
);
}
function Child() {
const id = useId();
return <div>{id}</div>;
}

function App({showMore}) {
return (
<React.StrictMode>
<Parent />
</React.StrictMode>
);
}

await serverAct(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
pipe(writable);
});
expect(container).toMatchInlineSnapshot(`
<div
id="container"
>
<div>
:R0:
<!-- -->
<div>
:R7:
</div>
</div>
</div>
`);

await clientAct(async () => {
ReactDOMClient.hydrateRoot(container, <App />);
});
expect(container).toMatchInlineSnapshot(`
<div
id="container"
>
<div>
:R0:
<!-- -->
<div>
:R7:
</div>
</div>
</div>
`);
});
});
1 change: 0 additions & 1 deletion packages/react-reconciler/src/ReactFiberHooks.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,6 @@ function renderWithHooksAgain<Props, SecondArg>(
let children;
do {
didScheduleRenderPhaseUpdateDuringThisPass = false;
localIdCounter = 0;
thenableIndexCounter = 0;

if (numberOfReRenders >= RE_RENDER_LIMIT) {
Expand Down
1 change: 0 additions & 1 deletion packages/react-reconciler/src/ReactFiberHooks.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,6 @@ function renderWithHooksAgain<Props, SecondArg>(
let children;
do {
didScheduleRenderPhaseUpdateDuringThisPass = false;
localIdCounter = 0;
thenableIndexCounter = 0;

if (numberOfReRenders >= RE_RENDER_LIMIT) {
Expand Down

0 comments on commit 15557fa

Please sign in to comment.