[Fix] properly track useId use in StrictMode in development (#25713)

gnoff · web-flow · commit 15557fa67fe3 · 2022-11-28T09:42:57.000-08:00
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.
diff --git a/packages/react-dom/src/__tests__/ReactDOMUseId-test.js b/packages/react-dom/src/__tests__/ReactDOMUseId-test.js
@@ -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>
+    `);
+  });
 });
diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js
@@ -697,7 +697,6 @@ function renderWithHooksAgain<Props, SecondArg>(
   let children;
   do {
     didScheduleRenderPhaseUpdateDuringThisPass = false;
-    localIdCounter = 0;
     thenableIndexCounter = 0;
 
     if (numberOfReRenders >= RE_RENDER_LIMIT) {
diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js
@@ -697,7 +697,6 @@ function renderWithHooksAgain<Props, SecondArg>(
   let children;
   do {
     didScheduleRenderPhaseUpdateDuringThisPass = false;
-    localIdCounter = 0;
     thenableIndexCounter = 0;
 
     if (numberOfReRenders >= RE_RENDER_LIMIT) {
