support React 18 #122

yen-tt · 2022-05-23T17:05:36Z

This pr updates answers-headless-react to support React 18 as well as 17.

use alpha version 1.1.1.-alpha.0-95 of answers-headless which contains redux package update to support React 18.
to support react 18 concurrent mode, useSyncExternalStore is needed. This pr uses use-sync-external-store package for backwards-compatible with pre-18 versions (v17).
- From internal docs in use-sync-external-store, the package does not offer much logic to support the case of server side rendering for react pre-18 versions. So, this pr update useAnswersState to be conditionally exported with either useAnswersStateWithManualStoreSync for SSR pre-18 or useAnswersStateWithReactStoreSync for everything else. updated to use use-sync-external-store as is, without the manual handling, for all cases (for more info).
- with v17 version, there's warnings for state update on unmounted components, which we do get for when an async function (execute universal/vertical search) finish and send a series of dispatch that would trigger a re-render of a component that's already unmounted. There's no actual memory leak here since we always unsubscribe out listeners. This warning was removed in v18 (visit this PR for more details). As a workaround, isMountedRef is used in useAnswersStateWithReactStoreSync to track when a component is unmounted and prevent the callback for handlling store changes from triggering (cause component update).
- update relevant packages to support v18 as well (@testing-library/react, @types/react)

Note: will release an alpha version of this for the component lib.

J=SLAP-2095
TEST=manual&auto

added new jest tests for server side rendering and hydration process.
see that jest tests passed successfully for V17 and V18. (note that @testing-library/react v13 drop support for 17 when beginning to support 18 in the same version)
test (client side) with React version 17 and React version 18 in test-site from component lib repo.
- with React verison 18. test with render (pre-18 syntax) and createRoot (18), ran a couple search and switch between universal and vertical page. see that both works as expected, although there is a warning for the former approach (Warning: ReactDOM.render is no longer supported in React 18. Use createRoot instead. Until you switch to the new API, your app will behave as if it's running React 17.) which is expected.
test (SSR) with React version 17 and React version 18 with yext-site-starters repo. See that the assets were built successfully and the page function as expected.

…y notices check

coveralls · 2022-05-23T17:56:22Z

Coverage decreased (-6.4%) to 63.333% when pulling b0459bb on dev/support-react-18 into dcd0f41 on develop.

package.json

src/AnswersHeadlessProvider.tsx

oshi97 · 2022-05-23T20:17:42Z

src/useAnswersState.tsx

+ */
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+export const useAnswersState = (React as any).useSyncExternalStore === undefined && isServerEnvironment
+  ? useAnswersStateWithManualStoreSync : useAnswersStateWithReactStoreSync;


so useAnswersStateWithManualStoreSync is only for react 17 SSR?

does useAnswersStateWithManualStoreSync work with react 18 as well?

I'm also kind of curious if you read anything about what useSyncExternalStore is doing under the hood? If not then nbd

When I was testing with startTransition to try to simulate tearing issues, useAnswersStateWithManualStoreSync doesn't work well with concurrent feature in react 18. Yup! useAnswersStateWithManualStoreSync would be for react 17 SSR only since use-sync-external-store library can handle the client side rendering for 17 and automatically use the builtin useSyncExternalStore API for 18.
I didn't dig too much into what useSyncExternalStore does internally but it looks like it is a reform of useMutableSource if you are familiar with that

src/useAnswersState.tsx

oshi97 · 2022-05-24T14:54:55Z

Have we tried looking at what react-redux is doing? It looks like we match up pretty closely? https://github.com/reduxjs/react-redux/blob/master/src/hooks/useSelector.ts
It looks like they support react 16 and 17 though so I'm kind of curious if you have any idea how they're doing so? I haven't looked suuuper closely but it looks like they're also doing some kind of conditional export with src/index.ts and src/next.ts
https://github.com/reduxjs/react-redux/blob/master/package.json

…y notices check

yen-tt · 2022-05-24T21:38:13Z

Have we tried looking at what react-redux is doing? It looks like we match up pretty closely? https://github.com/reduxjs/react-redux/blob/master/src/hooks/useSelector.ts It looks like they support react 16 and 17 though so I'm kind of curious if you have any idea how they're doing so? I haven't looked suuuper closely but it looks like they're also doing some kind of conditional export with src/index.ts and src/next.ts https://github.com/reduxjs/react-redux/blob/master/package.json

After some digging into SSR, it look like component will NOT re-render so the logic to handle memoization of state selector and register subscription (useAnswersStateWithManualSyncStore) is not necessary - useEffect, which is used in useAnswersStateWithManualSyncStore, will not run in server. This make sense with the way that use-sync-external-store and react-redux also doesn't do much with this case of SSR pre-v18 other than directly returning the state snapshot. This would mean we need a way to apply initial state to redux BEFORE the call renderToString of the component in ssr, which would require some changes to AnswersHeadlessProvider and configureStore in answers-headless to accept a preloadedState option.

package.json

src/useAnswersState.tsx

tests/useAnswersState.test.tsx

oshi97 · 2022-05-25T14:15:52Z

tests/useAnswersState.test.tsx

@@ -69,15 +101,15 @@ it('does not perform extra renders/listener registrations for nested components'
  expect(childStateUpdates).toHaveLength(0);

  userEvent.click(screen.getByText('Search'));
-  await pendingVerticalQuery;
+  await act( async () => { await pendingVerticalQuery; });


out of curiosity why are these acts necessary?

I remember getting some warning about When testing, code that causes React state updates should be wrapped into act(...). I think we only mock the response of executeVerticalQuery and not the function itself, so it still would send dispatch calls that trigger state updates? I wasn't getting the right number of times state listeners and updates without act(..) wrapper. We do this for the other tests, just not in this one

Oh ok, maybe this is due to changing from react 17 to 18 with how batching works or something. I think I wrote this original test in a pretty weird way but I couldn't think of an easy way to change it here so eh

src/useAnswersState.tsx

## Version 1.2.0 ### Features - Added support for React 16.14 and React 18 (#122, #124) - Export Sandbox endpoints (#125)

support react 18

50b5f94

yen-tt requested a review from a team as a code owner May 23, 2022 17:05

github-actions bot and others added 2 commits May 23, 2022 17:06

Automated update to THIRD-PARTY-NOTICES from github action's 3rd part…

266baf1

…y notices check

update package version

188a716

oshi97 reviewed May 23, 2022

View reviewed changes

package.json Outdated Show resolved Hide resolved

oshi97 reviewed May 23, 2022

View reviewed changes

src/AnswersHeadlessProvider.tsx Show resolved Hide resolved

oshi97 reviewed May 23, 2022

View reviewed changes

src/useAnswersState.tsx Outdated Show resolved Hide resolved

remove .0 from package version. nit

189c762

yen-tt requested review from oshi97 and a team May 24, 2022 13:53

oshi97 reviewed May 24, 2022

View reviewed changes

src/useAnswersState.tsx Outdated Show resolved Hide resolved

oshi97 reviewed May 24, 2022

View reviewed changes

src/useAnswersState.tsx Outdated Show resolved Hide resolved

oshi97 reviewed May 24, 2022

View reviewed changes

src/useAnswersState.tsx Outdated Show resolved Hide resolved

Yen Truong and others added 2 commits May 24, 2022 17:31

add ssr tests, remove manualSyncStore

45bb749

Automated update to THIRD-PARTY-NOTICES from github action's 3rd part…

5429009

…y notices check

yen-tt requested a review from oshi97 May 24, 2022 21:40

remove unneeded checks

64dcc67

oshi97 reviewed May 25, 2022

View reviewed changes

package.json Outdated Show resolved Hide resolved

oshi97 reviewed May 25, 2022

View reviewed changes

src/useAnswersState.tsx Outdated Show resolved Hide resolved

oshi97 reviewed May 25, 2022

View reviewed changes

tests/useAnswersState.test.tsx Outdated Show resolved Hide resolved

oshi97 reviewed May 25, 2022

View reviewed changes

src/useAnswersState.tsx Show resolved Hide resolved

oshi97 reviewed May 25, 2022

View reviewed changes

src/useAnswersState.tsx Show resolved Hide resolved

Yen Truong added 3 commits May 25, 2022 11:12

feedback

a6e0e3a

formatting

bbe211e

wording

b0459bb

yen-tt requested a review from oshi97 May 25, 2022 15:17

oshi97 approved these changes May 25, 2022

View reviewed changes

yen-tt merged commit 6848583 into develop May 25, 2022

yen-tt deleted the dev/support-react-18 branch May 25, 2022 18:20

nmanu1 mentioned this pull request Jun 16, 2022

Version 1.2.0 #131

Merged

nmanu1 added a commit that referenced this pull request Jun 16, 2022

Version 1.2.0 (#131)

88e48dd

## Version 1.2.0 ### Features - Added support for React 16.14 and React 18 (#122, #124) - Export Sandbox endpoints (#125)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support React 18 #122

support React 18 #122

yen-tt commented May 23, 2022 •

edited

Loading

coveralls commented May 23, 2022 •

edited

Loading

oshi97 May 23, 2022

oshi97 May 23, 2022

oshi97 May 23, 2022

yen-tt May 24, 2022 •

edited

Loading

oshi97 commented May 24, 2022 •

edited

Loading

yen-tt commented May 24, 2022 •

edited

Loading

oshi97 May 25, 2022

yen-tt May 25, 2022

oshi97 May 25, 2022

support React 18 #122

support React 18 #122

Conversation

yen-tt commented May 23, 2022 • edited Loading

coveralls commented May 23, 2022 • edited Loading

oshi97 May 23, 2022

Choose a reason for hiding this comment

oshi97 May 23, 2022

Choose a reason for hiding this comment

oshi97 May 23, 2022

Choose a reason for hiding this comment

yen-tt May 24, 2022 • edited Loading

Choose a reason for hiding this comment

oshi97 commented May 24, 2022 • edited Loading

yen-tt commented May 24, 2022 • edited Loading

oshi97 May 25, 2022

Choose a reason for hiding this comment

yen-tt May 25, 2022

Choose a reason for hiding this comment

oshi97 May 25, 2022

Choose a reason for hiding this comment

yen-tt commented May 23, 2022 •

edited

Loading

coveralls commented May 23, 2022 •

edited

Loading

yen-tt May 24, 2022 •

edited

Loading

oshi97 commented May 24, 2022 •

edited

Loading

yen-tt commented May 24, 2022 •

edited

Loading