From bc4569fc919d028e5a460fe844ce9d1418d61414 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Mon, 1 Nov 2021 22:34:03 -0400 Subject: [PATCH 1/4] Add tentative SSR support for useSelector --- src/components/Context.ts | 1 + src/components/Provider.tsx | 16 ++++++++++++---- src/hooks/useSelector.ts | 4 ++-- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/components/Context.ts b/src/components/Context.ts index fa9c86216..1a7ea88d3 100644 --- a/src/components/Context.ts +++ b/src/components/Context.ts @@ -8,6 +8,7 @@ export interface ReactReduxContextValue< > { store: Store subscription: Subscription + getServerState?: () => SS } export const ReactReduxContext = diff --git a/src/components/Provider.tsx b/src/components/Provider.tsx index f1c12fe10..364ff7bc8 100644 --- a/src/components/Provider.tsx +++ b/src/components/Provider.tsx @@ -4,17 +4,23 @@ import { createSubscription } from '../utils/Subscription' import { useIsomorphicLayoutEffect } from '../utils/useIsomorphicLayoutEffect' import { Action, AnyAction, Store } from 'redux' -export interface ProviderProps { +export interface ProviderProps { /** * The single Redux store in your application. */ - store: Store + store: Store + + /** + * An optional server state snapshot. Will be used during initial hydration render if available, to ensure that the UI output is consistent with the HTML generated on the server. + */ + serverState?: S + /** * Optional context to be used internally in react-redux. Use React.createContext() to create a context to be used. * If this is used, you'll need to customize `connect` by supplying the same context provided to the Provider. * Initial value doesn't matter, as it is overwritten with the internal state of Provider. */ - context?: Context> + context?: Context> children: ReactNode } @@ -22,14 +28,16 @@ function Provider({ store, context, children, + serverState, }: ProviderProps) { const contextValue = useMemo(() => { const subscription = createSubscription(store) return { store, subscription, + getServerState: serverState ? () => serverState : undefined, } - }, [store]) + }, [store, serverState]) const previousState = useMemo(() => store.getState(), [store]) diff --git a/src/hooks/useSelector.ts b/src/hooks/useSelector.ts index 2a93f941e..e05525be3 100644 --- a/src/hooks/useSelector.ts +++ b/src/hooks/useSelector.ts @@ -48,13 +48,13 @@ export function createSelectorHook( } } - const { store } = useReduxContext()! + const { store, getServerState } = useReduxContext()! const selectedState = useSyncExternalStoreWithSelector( store.subscribe, store.getState, // TODO Need a server-side snapshot here - store.getState, + getServerState || store.getState, selector, equalityFn ) From 63f626b3b0703c4b7de658dce7c319a8ed3c7ee2 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Mon, 1 Nov 2021 22:34:23 -0400 Subject: [PATCH 2/4] Comment areas of concern for `connect` + `uSES` --- src/components/connect.tsx | 14 +++++++++++++- src/utils/useIsomorphicLayoutEffect.ts | 8 +++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/components/connect.tsx b/src/components/connect.tsx index f4eb8da80..fad8e6b22 100644 --- a/src/components/connect.tsx +++ b/src/components/connect.tsx @@ -27,7 +27,10 @@ import defaultMapStateToPropsFactories from '../connect/mapStateToProps' import defaultMergePropsFactories from '../connect/mergeProps' import { createSubscription, Subscription } from '../utils/Subscription' -import { useIsomorphicLayoutEffect } from '../utils/useIsomorphicLayoutEffect' +import { + useIsomorphicLayoutEffect, + canUseDOM, +} from '../utils/useIsomorphicLayoutEffect' import shallowEqual from '../utils/shallowEqual' import { @@ -124,6 +127,7 @@ function subscribeUpdates( return } + // TODO We're currently calling getState ourselves here, rather than letting `uSES` do it const latestStoreState = store.getState() let newChildProps, error @@ -157,6 +161,7 @@ function subscribeUpdates( childPropsFromStoreUpdate.current = newChildProps renderIsScheduled.current = true + // TODO This is hacky and not how `uSES` is meant to be used // Trigger the React `useSyncExternalStore` subscriber additionalSubscribeListener() } @@ -597,6 +602,10 @@ function connect< ? props.store! : contextValue!.store + const getServerSnapshot = didStoreComeFromContext + ? contextValue.getServerState + : store.getState + const childPropsSelector = useMemo(() => { // The child props selector needs the store reference as an input. // Re-create this selector whenever the store changes. @@ -724,7 +733,10 @@ function connect< try { actualChildProps = useSyncExternalStore( + // TODO We're passing through a big wrapper that does a bunch of extra side effects besides subscribing subscribeForReact, + // TODO This is incredibly hacky. We've already processed the store update and calculated new child props, + // TODO and we're just passing that through so it triggers a re-render for us rather than relying on `uSES`. actualChildPropsSelector, // TODO Need a real getServerSnapshot here actualChildPropsSelector diff --git a/src/utils/useIsomorphicLayoutEffect.ts b/src/utils/useIsomorphicLayoutEffect.ts index 0e87d6e0c..d60f51c2e 100644 --- a/src/utils/useIsomorphicLayoutEffect.ts +++ b/src/utils/useIsomorphicLayoutEffect.ts @@ -9,9 +9,11 @@ import { useEffect, useLayoutEffect } from 'react' // is created synchronously, otherwise a store update may occur before the // subscription is created and an inconsistent state may be observed -export const useIsomorphicLayoutEffect = +// Matches logic in React's `shared/ExecutionEnvironment` file +export const canUseDOM = !!( typeof window !== 'undefined' && typeof window.document !== 'undefined' && typeof window.document.createElement !== 'undefined' - ? useLayoutEffect - : useEffect +) + +export const useIsomorphicLayoutEffect = canUseDOM ? useLayoutEffect : useEffect From ff4f9d12fa303ccac1e6145f32a06d4743b66848 Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Wed, 22 Dec 2021 16:32:36 -0500 Subject: [PATCH 3/4] Update React deps to 18 RC.0 and fix wrongly added devDeps --- package.json | 14 +++++++------- yarn.lock | 25 ++++++++++++++++--------- 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/package.json b/package.json index 788b7ff37..b2b195f37 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,7 @@ "coverage": "codecov" }, "peerDependencies": { - "react": "^18.0.0-beta" + "react": "^18.0.0-rc" }, "peerDependenciesMeta": { "react-dom": { @@ -52,15 +52,11 @@ }, "dependencies": { "@babel/runtime": "^7.12.1", - "@testing-library/react-12": "npm:@testing-library/react@^12", "@types/hoist-non-react-statics": "^3.3.1", "@types/use-sync-external-store": "^0.0.3", "hoist-non-react-statics": "^3.3.2", - "react-17": "npm:react@^17", - "react-dom-17": "npm:react-dom@^17", - "react-is": "^18.0.0-beta-fdc1d617a-20211118", - "react-test-renderer-17": "npm:react-test-renderer@^17", - "use-sync-external-store": "1.0.0-beta-fdc1d617a-20211118" + "react-is": "^18.0.0-rc.0", + "use-sync-external-store": "^1.0.0-rc.0" }, "devDependencies": { "@babel/cli": "^7.12.1", @@ -81,6 +77,7 @@ "@testing-library/jest-dom": "^5.11.5", "@testing-library/jest-native": "^3.4.3", "@testing-library/react": "13.0.0-alpha.4", + "@testing-library/react-12": "npm:@testing-library/react@^12", "@testing-library/react-hooks": "^3.4.2", "@testing-library/react-native": "^7.1.0", "@types/object-assign": "^4.0.30", @@ -104,9 +101,12 @@ "jest": "^26.6.1", "prettier": "^2.1.2", "react": "18.0.0-beta-fdc1d617a-20211118", + "react-17": "npm:react@^17", "react-dom": "18.0.0-beta-fdc1d617a-20211118", + "react-dom-17": "npm:react-dom@^17", "react-native": "^0.64.1", "react-test-renderer": "18.0.0-beta-fdc1d617a-20211118", + "react-test-renderer-17": "npm:react-test-renderer@^17", "redux": "^4.0.5", "rimraf": "^3.0.2", "rollup": "^2.32.1", diff --git a/yarn.lock b/yarn.lock index 9f0aa938d..016568113 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8837,7 +8837,7 @@ __metadata: languageName: node linkType: hard -"react-is@npm:18.0.0-beta-fdc1d617a-20211118, react-is@npm:^18.0.0-beta-fdc1d617a-20211118": +"react-is@npm:18.0.0-beta-fdc1d617a-20211118": version: 18.0.0-beta-fdc1d617a-20211118 resolution: "react-is@npm:18.0.0-beta-fdc1d617a-20211118" checksum: 402e28c80019e9deaee919c4373606736ea36142d1d1e09e0cab89d52e8bd647f1176a383987cca5c4e83bfee33bba8091ab347363ea4bdd4f9c3a707812c8b9 @@ -8858,6 +8858,13 @@ __metadata: languageName: node linkType: hard +"react-is@npm:^18.0.0-rc.0": + version: 18.0.0-rc.0-next-f2a59df48-20211208 + resolution: "react-is@npm:18.0.0-rc.0-next-f2a59df48-20211208" + checksum: e0f8b1c8ef759153072b7508f8832164651aa855c9c939d607090899d9d522c68ad12ec9630f34a56c46330542356207dfc09d744b8ffe2eccc45a83f296fcae + languageName: node + linkType: hard + "react-native-codegen@npm:^0.0.6": version: 0.0.6 resolution: "react-native-codegen@npm:0.0.6" @@ -8966,7 +8973,7 @@ __metadata: react-17: "npm:react@^17" react-dom: 18.0.0-beta-fdc1d617a-20211118 react-dom-17: "npm:react-dom@^17" - react-is: ^18.0.0-beta-fdc1d617a-20211118 + react-is: ^18.0.0-rc.0 react-native: ^0.64.1 react-test-renderer: 18.0.0-beta-fdc1d617a-20211118 react-test-renderer-17: "npm:react-test-renderer@^17" @@ -8976,9 +8983,9 @@ __metadata: rollup-plugin-terser: ^7.0.2 ts-jest: 26.5.6 typescript: ^4.3.4 - use-sync-external-store: 1.0.0-beta-fdc1d617a-20211118 + use-sync-external-store: ^1.0.0-rc.0 peerDependencies: - react: ^18.0.0-beta + react: ^18.0.0-rc peerDependenciesMeta: react-dom: optional: true @@ -10845,12 +10852,12 @@ __metadata: languageName: node linkType: hard -"use-sync-external-store@npm:1.0.0-beta-fdc1d617a-20211118": - version: 1.0.0-beta-fdc1d617a-20211118 - resolution: "use-sync-external-store@npm:1.0.0-beta-fdc1d617a-20211118" +"use-sync-external-store@npm:^1.0.0-rc.0": + version: 1.0.0-rc.0-next-f2a59df48-20211208 + resolution: "use-sync-external-store@npm:1.0.0-rc.0-next-f2a59df48-20211208" peerDependencies: - react: 18.0.0-beta-fdc1d617a-20211118 - checksum: bb52d87a886731595163a3b6a07b671f9ea92ca6de35791f32ce186e2fea0f2989088f3dacfea3dff258e866e767ff1e87d18494e436523e67b17e59051a22e0 + react: 18.0.0-rc.0-next-f2a59df48-20211208 + checksum: 772bf9fe4b47f9cf21969cc839e21e11e2fbad63e4992c75402fabed7048f5afb559bee289c9a0c3b4bad7c1c5724f6cb63c5499ef7f249c395e86233102bccb languageName: node linkType: hard From ed8fdc8407b9b478c37aaca2e3d819f83e6a539f Mon Sep 17 00:00:00 2001 From: Mark Erikson Date: Wed, 22 Dec 2021 16:33:50 -0500 Subject: [PATCH 4/4] Actually use server state for hydration in `connect` if available --- src/components/connect.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/components/connect.tsx b/src/components/connect.tsx index fad8e6b22..e9239aa49 100644 --- a/src/components/connect.tsx +++ b/src/components/connect.tsx @@ -602,7 +602,7 @@ function connect< ? props.store! : contextValue!.store - const getServerSnapshot = didStoreComeFromContext + const getServerState = didStoreComeFromContext ? contextValue.getServerState : store.getState @@ -738,8 +738,9 @@ function connect< // TODO This is incredibly hacky. We've already processed the store update and calculated new child props, // TODO and we're just passing that through so it triggers a re-render for us rather than relying on `uSES`. actualChildPropsSelector, - // TODO Need a real getServerSnapshot here - actualChildPropsSelector + getServerState + ? () => childPropsSelector(getServerState(), wrapperProps) + : actualChildPropsSelector ) } catch (err) { if (latestSubscriptionCallbackError.current) {