Fix React SSR hydration ID mismatch error #315

yen-tt · 2022-10-14T19:33:25Z

This PR is to fix the errors Warning: Prop "id" did not match. Server: "some-id". Client: "another-id", reported in https://yext.slack.com/archives/C032CKFARGS/p1664916209295449

React expects that the rendered content is identical between the server and the client. However, our usage of uuid means different id is generated from the HTML content generated through SSR vs the first render in client side during hydration process. This is a known issues in React SSR (issues in react repo[1][2][3]) and React released a new hook useId in React 18 to address it.

I found three potential solutions for React <18 to generate unique but predictable id, each with an external library we could use:

Use context to keep a counter in state, so counter is not shared across renders. This approach is implemented in react-aria. When using SSR with React Aria, applications must be wrapped in an SSRProvider component to reset the count in server side to be zero for each fresh render. Multiple copies of React Aria is not supported so it would be a peer-dep

//in component lib (dropdown)
import { useId } from 'react-aria';
...
const screenReaderUUID: string = useId();

//in pageJS template using SSR
import { SSRProvider } from 'react-aria';
...
<SSRProvider>
    <TestComponent />
</SSRProvider>

use a global counter to increment on initial renders and expose a reset function for SSR side. This approach is implemented in react-id-generator. There's a function resetId required to invoke from SSR side (such as in PageJS serverRenderRoute or somewhere in user's template page) to avoid mismatch during refresh, since browser generator will always restart from "1" but server's count will keep incrementing.
Don't use IDs at all on the server; patch after first render. This approach is implemented in reach/auto-id. ID returned as undefined/empty on the first render so server and client have the same markup during hydration process. After render, patch the components with an incremented ID (second render) in useLayoutEffect. (more info here) No changes needed outside of using import { useId } from '@reach/auto-id'; in our component lib.

I decided to go with the last approach using react/auto-id because:

requires no additional work / changes from user side (only in component lib)
contain fallback implementation to React 18 useId if it's available
avoid potential concurrent rendering problem (such as suspense boundaries)
the main issue with this approach is that it causes a double render on any components with useId. However, since we (1) only update the ID attribute on DOM on second render, and (2) our components that uses useId already requires multiple renders to get setup, the performance is not greatly affected. I used Profiler in our test-site to measure performance, the number of renders and time spent for SearchBar, StaticFilters, and FilterSearch did not change as the dispatch from useId is grouped with other changes required in our components on second render. This is only needed for React <18

J=SLAP-2403
TEST=manual&auto

npm pack a build of the component lib with the new changes and tested SearchBar, FilterSearch, and StaticFilers:

in the starter PageJS project (https://github.com/yext/pages-starter-react-locations) with React 17
in another yext-sites-starter repo with React 18 override

See that the warning about Prop id mismatched no longer appear in console

added new jest test for Dropdown and StaticFilter, see that test failed the previous usage of uuid, and passed with reach/auto-id.

…nt and server

coveralls · 2022-10-14T19:36:55Z

Coverage increased (+0.05%) to 85.401% when pulling 34f950f on dev/fix-id-gen-ssr-issue into d8e76b9 on develop.

github-actions · 2022-10-17T15:25:43Z

Current unit coverage is 88.49693251533742%
Current visual coverage is 77.40899357601712%
Current combined coverage is 89.03374233128834%

oshi97 · 2022-10-17T16:55:15Z

.storybook/main.js

@@ -30,6 +30,10 @@ module.exports = {
  framework: '@storybook/react',
  staticDirs: ['./public'],
  webpackFinal: async (config) => {
+    //use commonjs entry point for "@reach" packages
+    config.resolve.alias['@reach/auto-id'] = require.resolve('@reach/auto-id');


out of curiosity why is this necessary?

our storybook uses commonjs and doesn't seem to handle mjs files.

@reach/auto-id does export a commonjs version. But its package.json is set to "module": "./dist/reach-auto-id.mjs", and "main": "./dist/reach-auto-id.cjs.js",. I believe Webpack, used by Storybook, by default will try to use "module" over "main". Alternatively, we could also do config.resolve.mainFields = ['main', 'module']; but I thought the alias approach would be more specific.

src/components/Dropdown/Dropdown.tsx

tatimblin · 2022-10-17T17:21:30Z

tests/ssr/utils.tsx

+  render(<App />, { container, hydrate: true });
+  expect(consoleErrorSpy).not.toBeCalledWith(
+    expect.stringContaining('Warning: Prop `%s` did not match. Server: %s Client: %s%s'),
+    'id',


don't need to use string concatenation for a hardcoded value

removed expect.stringContaining, the rest of the arguments is needed in the order of the params received by console error.

I think here it's react itself that's calling console.error . They probably use the same error call for all props which is why there's the string formatting bit there.
If react changes the implementation of their console.error call this test will stop functioning properly right? is there some other way to test this?

yea, all prop mismatched error will use this message format. I suppose we don't need to explicitly check for id mismatched only, although I don't see other ways of testing this beside checking console error. I attempted to check for the html content of server vs client but it didn't work -- the actual of the container after hydration uses the same as server even though the error is present.
I decided to update the test to assert the number of error logged outside of the useLayoutEffect warning. This way, they may change their message format, the test will still fail as long as there is an error logged.

Sure that makes sense, I can't think of a better way to test this. If they switch to console.warn for example maybe we start getting false positives from the test, but we'd also notice console.warn's in the jest output (hopefully). If they remove the console messages then 🤷 but seems unlikely

### Features - Default behavior of `FilterSearch` was changed to better support Locators and Doctor Finders. Additionally, a new `onSelect` prop was added to the Component. The `searchOnSelect` prop is now deprecated. (#323, #343, #333) - A new CSS bundle without the Tailwind resets is exported. (#322) - We've added a `MapboxMap` Component, powered by v2 of their JavaScript API. (#332) ### Changes - Assorted updates to improve our GH Actions. - Styling of Facet Headers is now exposed in `FilterGroupCssClasses`. (#321) ### Bug Fixes - Vulnerabilities were addressed for the repo and its test-site. - Fixed the Dropdown Component to invoke `preventDefault` only when it is active. (#307) - Corrected a small error in the generation of SSR Hydration IDs. (#315)

deterministic id generation for identical render content between clie…

32c3764

…nt and server

yen-tt requested a review from a team as a code owner October 14, 2022 19:33

remove test changes

4ee9a31

add uuid for test-site, fix storybook issue

8a2418b

yen-tt force-pushed the dev/fix-id-gen-ssr-issue branch from a00fc0e to 8a2418b Compare October 17, 2022 15:21

update notice

0974432

Yen Truong added 2 commits October 17, 2022 12:04

docs

52b2dc8

switch to regex

0a47502

oshi97 reviewed Oct 17, 2022

View reviewed changes

src/components/Dropdown/Dropdown.tsx Show resolved Hide resolved

tatimblin reviewed Oct 17, 2022

View reviewed changes

Yen Truong added 2 commits October 17, 2022 13:42

string match

0530610

count number of errors logged instead

8dba976

yen-tt requested a review from oshi97 October 17, 2022 19:43

oshi97 approved these changes Oct 17, 2022

View reviewed changes

refactor util input

34f950f

oshi97 approved these changes Oct 17, 2022

View reviewed changes

yen-tt merged commit 3b0ecbf into develop Oct 17, 2022

yen-tt deleted the dev/fix-id-gen-ssr-issue branch October 17, 2022 20:16

tmeyer2115 mentioned this pull request Dec 15, 2022

v1.1.0 #347

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix React SSR hydration ID mismatch error #315

Fix React SSR hydration ID mismatch error #315

yen-tt commented Oct 14, 2022 •

edited

Loading

coveralls commented Oct 14, 2022 •

edited

Loading

github-actions bot commented Oct 17, 2022

oshi97 Oct 17, 2022

yen-tt Oct 17, 2022 •

edited

Loading

tatimblin Oct 17, 2022

yen-tt Oct 17, 2022 •

edited

Loading

oshi97 Oct 17, 2022 •

edited

Loading

yen-tt Oct 17, 2022 •

edited

Loading

oshi97 Oct 17, 2022

Fix React SSR hydration ID mismatch error #315

Fix React SSR hydration ID mismatch error #315

Conversation

yen-tt commented Oct 14, 2022 • edited Loading

coveralls commented Oct 14, 2022 • edited Loading

github-actions bot commented Oct 17, 2022

oshi97 Oct 17, 2022

Choose a reason for hiding this comment

yen-tt Oct 17, 2022 • edited Loading

Choose a reason for hiding this comment

tatimblin Oct 17, 2022

Choose a reason for hiding this comment

yen-tt Oct 17, 2022 • edited Loading

Choose a reason for hiding this comment

oshi97 Oct 17, 2022 • edited Loading

Choose a reason for hiding this comment

yen-tt Oct 17, 2022 • edited Loading

Choose a reason for hiding this comment

oshi97 Oct 17, 2022

Choose a reason for hiding this comment

yen-tt commented Oct 14, 2022 •

edited

Loading

coveralls commented Oct 14, 2022 •

edited

Loading

yen-tt Oct 17, 2022 •

edited

Loading

yen-tt Oct 17, 2022 •

edited

Loading

oshi97 Oct 17, 2022 •

edited

Loading

yen-tt Oct 17, 2022 •

edited

Loading