Merge pull request #1842 from reduxjs/feature/uses-compat-shim

markerikson · web-flow · commit 19ae75e52cef · 2021-11-18T23:23:57.000-05:00
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -40,7 +40,7 @@ jobs:
       fail-fast: false
       matrix:
         node: ['14.x']
-        ts: ['3.9', '4.0', '4.1', '4.2', '4.3', 'next']
+        ts: ['4.0', '4.1', '4.2', '4.3', '4.4', '4.5', 'next']
     steps:
       - name: Checkout repo
         uses: actions/checkout@v2
diff --git a/jest.config.js b/jest.config.js
@@ -29,6 +29,18 @@ const rnConfig = {
   },
 }
 
+const compatEntryConfig = {
+  ...tsStandardConfig,
+  displayName: 'Compat',
+  moduleNameMapper: {
+    '^react$': 'react-17',
+    '^react-dom$': 'react-dom-17',
+    '^react-test-renderer$': 'react-test-renderer-17',
+    '^@testing-library/react$': '@testing-library/react-12',
+    '../../src/index': '<rootDir>/src/compat',
+  },
+}
+
 module.exports = {
-  projects: [tsStandardConfig, rnConfig],
+  projects: [tsStandardConfig, rnConfig, compatEntryConfig],
 }
diff --git a/package.json b/package.json
@@ -40,7 +40,7 @@
     "coverage": "codecov"
   },
   "peerDependencies": {
-    "react": "^18.0.0-alpha || ^18.0.0-beta"
+    "react": "^18.0.0-beta"
   },
   "peerDependenciesMeta": {
     "react-dom": {
@@ -52,12 +52,15 @@
   },
   "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.0",
+    "@types/use-sync-external-store": "^0.0.3",
     "hoist-non-react-statics": "^3.3.2",
-    "loose-envify": "^1.4.0",
-    "react-is": "^16.13.1",
-    "use-sync-external-store": "1.0.0-alpha-5cccacd13-20211101"
+    "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"
   },
   "devDependencies": {
     "@babel/cli": "^7.12.1",
@@ -80,10 +83,9 @@
     "@testing-library/react": "13.0.0-alpha.4",
     "@testing-library/react-hooks": "^3.4.2",
     "@testing-library/react-native": "^7.1.0",
-    "@types/create-react-class": "^15.6.3",
     "@types/object-assign": "^4.0.30",
-    "@types/react": "17.0.19",
-    "@types/react-dom": "^17.0.9",
+    "@types/react": "^17.0.35",
+    "@types/react-dom": "^17.0.11",
     "@types/react-is": "^17.0.1",
     "@types/react-native": "^0.64.12",
     "@types/react-redux": "^7.1.18",
@@ -92,9 +94,7 @@
     "babel-eslint": "^10.1.0",
     "babel-jest": "^26.6.1",
     "codecov": "^3.8.0",
-    "create-react-class": "^15.7.0",
     "cross-env": "^7.0.2",
-    "es3ify": "^0.2.0",
     "eslint": "^7.12.0",
     "eslint-config-prettier": "^6.14.0",
     "eslint-plugin-import": "^2.22.1",
@@ -103,20 +103,15 @@
     "glob": "^7.1.6",
     "jest": "^26.6.1",
     "prettier": "^2.1.2",
-    "react": "18.0.0-alpha-5cccacd13-20211101",
-    "react-dom": "18.0.0-alpha-5cccacd13-20211101",
+    "react": "18.0.0-beta-fdc1d617a-20211118",
+    "react-dom": "18.0.0-beta-fdc1d617a-20211118",
     "react-native": "^0.64.1",
-    "react-test-renderer": "18.0.0-alpha-5cccacd13-20211101",
+    "react-test-renderer": "18.0.0-beta-fdc1d617a-20211118",
     "redux": "^4.0.5",
     "rimraf": "^3.0.2",
     "rollup": "^2.32.1",
     "rollup-plugin-terser": "^7.0.2",
     "ts-jest": "26.5.6",
     "typescript": "^4.3.4"
-  },
-  "browserify": {
-    "transform": [
-      "loose-envify"
-    ]
   }
 }
diff --git a/src/alternate-renderers.ts b/src/alternate-renderers.ts
@@ -1,4 +1,14 @@
-export * from './exports'
+// The "alternate renderers" entry point is primarily here to fall back on a no-op
+// version of `unstable_batchedUpdates`, for use with renderers other than ReactDOM/RN.
+// Examples include React-Three-Fiber, Ink, etc.
+// Because of that, we'll also assume the useSyncExternalStore compat shim is needed.
+
+import { useSyncExternalStore } from 'use-sync-external-store/shim'
+import { useSyncExternalStoreWithSelector } from 'use-sync-external-store/shim/with-selector'
+
+import { setSyncFunctions } from './utils/useSyncExternalStore'
+
+setSyncFunctions(useSyncExternalStore, useSyncExternalStoreWithSelector)
 
 import { getBatch } from './utils/batch'
 
@@ -7,3 +17,5 @@ import { getBatch } from './utils/batch'
 const batch = getBatch()
 
 export { batch }
+
+export * from './exports'
diff --git a/src/compat.ts b/src/compat.ts
@@ -0,0 +1,20 @@
+// The "compat" entry point assumes we're working with standard ReactDOM/RN, but
+// older versions that do not include `useSyncExternalStore` (React 16.9 - 17.x).
+// Because of that, the useSyncExternalStore compat shim is needed.
+
+import { useSyncExternalStore } from 'use-sync-external-store/shim'
+import { useSyncExternalStoreWithSelector } from 'use-sync-external-store/shim/with-selector'
+
+import { setSyncFunctions } from './utils/useSyncExternalStore'
+import { unstable_batchedUpdates as batch } from './utils/reactBatchedUpdates'
+import { setBatch } from './utils/batch'
+
+setSyncFunctions(useSyncExternalStore, useSyncExternalStoreWithSelector)
+
+// Enable batched updates in our subscriptions for use
+// with standard React renderers (ReactDOM, React Native)
+setBatch(batch)
+
+export { batch }
+
+export * from './exports'
diff --git a/src/components/Context.ts b/src/components/Context.ts
@@ -1,5 +1,5 @@
 import React from 'react'
-import { Action, AnyAction, Store } from 'redux'
+import type { Action, AnyAction, Store } from 'redux'
 import type { Subscription } from '../utils/Subscription'
 
 export interface ReactReduxContextValue<
diff --git a/src/components/connect.tsx b/src/components/connect.tsx
@@ -1,13 +1,6 @@
 /* eslint-disable valid-jsdoc, @typescript-eslint/no-unused-vars */
 import hoistStatics from 'hoist-non-react-statics'
-import React, {
-  useContext,
-  useMemo,
-  useRef,
-  useReducer,
-  // @ts-ignore
-  useSyncExternalStore,
-} from 'react'
+import React, { useContext, useMemo, useRef, useReducer } from 'react'
 import { isValidElementType, isContextConsumer } from 'react-is'
 
 import type { Store, Dispatch, Action, AnyAction } from 'redux'
@@ -35,6 +28,7 @@ import defaultMergePropsFactories from '../connect/mergeProps'
 
 import { createSubscription, Subscription } from '../utils/Subscription'
 import { useIsomorphicLayoutEffect } from '../utils/useIsomorphicLayoutEffect'
+import { getSyncFunctions } from '../utils/useSyncExternalStore'
 import shallowEqual from '../utils/shallowEqual'
 
 import {
@@ -43,6 +37,8 @@ import {
   ReactReduxContextInstance,
 } from './Context'
 
+const [useSyncExternalStore] = getSyncFunctions()
+
 // Define some constant arrays just to avoid re-creating these
 const EMPTY_ARRAY: [unknown, number] = [null, 0]
 const NO_SUBSCRIPTION_ARRAY = [null, null]
diff --git a/src/hooks/useSelector.ts b/src/hooks/useSelector.ts
@@ -1,11 +1,11 @@
 import { useContext, useDebugValue } from 'react'
 
-// @ts-ignore
-import { useSyncExternalStoreWithSelector } from 'use-sync-external-store/with-selector'
-
 import { useReduxContext as useDefaultReduxContext } from './useReduxContext'
 import { ReactReduxContext } from '../components/Context'
-import { DefaultRootState, EqualityFn } from '../types'
+import { getSyncFunctions } from '../utils/useSyncExternalStore'
+import type { DefaultRootState, EqualityFn } from '../types'
+
+const [, useSyncExternalStoreWithSelector] = getSyncFunctions()
 
 const refEquality: EqualityFn<any> = (a, b) => a === b
 
diff --git a/src/index.ts b/src/index.ts
@@ -1,10 +1,22 @@
-export * from './exports'
+// The default entry point assumes we are working with React 18, and thus have
+// useSyncExternalStore available. We can import that directly from React itself.
+// The useSyncExternalStoreWithSelector has to be imported, but we can use the
+// non-shim version. This shaves off the byte size of the shim.
+
+// @ts-ignore React types not updated yet
+import { useSyncExternalStore } from 'react'
+import { useSyncExternalStoreWithSelector } from 'use-sync-external-store/with-selector'
 
+import { setSyncFunctions } from './utils/useSyncExternalStore'
 import { unstable_batchedUpdates as batch } from './utils/reactBatchedUpdates'
 import { setBatch } from './utils/batch'
 
+setSyncFunctions(useSyncExternalStore, useSyncExternalStoreWithSelector)
+
 // Enable batched updates in our subscriptions for use
 // with standard React renderers (ReactDOM, React Native)
 setBatch(batch)
 
 export { batch }
+
+export * from './exports'
diff --git a/src/utils/useSyncExternalStore.ts b/src/utils/useSyncExternalStore.ts
@@ -0,0 +1,21 @@
+import type { useSyncExternalStore } from 'use-sync-external-store'
+import type { useSyncExternalStoreWithSelector } from 'use-sync-external-store/with-selector'
+
+const notInitialized = () => {
+  throw new Error('Not initialize!')
+}
+
+let uSES: typeof useSyncExternalStore = notInitialized
+let uSESWS: typeof useSyncExternalStoreWithSelector = notInitialized
+
+// Allow injecting the actual functions from the entry points
+export const setSyncFunctions = (
+  sync: typeof useSyncExternalStore,
+  withSelector: typeof useSyncExternalStoreWithSelector
+) => {
+  uSES = sync
+  uSESWS = withSelector
+}
+
+// Supply a getter just to skip dealing with ESM bindings
+export const getSyncFunctions = () => [uSES, uSESWS] as const
diff --git a/test/components/connect.spec.tsx b/test/components/connect.spec.tsx
@@ -1,7 +1,6 @@
 /*eslint-disable react/prop-types*/
 
 import React, { Component, MouseEvent } from 'react'
-import createClass from 'create-react-class'
 import { createStore, applyMiddleware } from 'redux'
 import { Provider as ProviderMock, connect } from '../../src/index'
 import * as rtl from '@testing-library/react'
@@ -15,6 +14,8 @@ import type {
 } from 'redux'
 import type { ReactReduxContextValue } from '../../src/index'
 
+const IS_REACT_18 = React.version.startsWith('18')
+
 describe('React', () => {
   describe('connect', () => {
     const propMapper = (prop: any): ReactNode => {
@@ -1842,31 +1843,6 @@ describe('React', () => {
             }
           ).displayName
         ).toBe('Connect(Foo)')
-
-        expect(
-          connect((state) => state)(
-            createClass({
-              displayName: 'Bar',
-              render() {
-                return <div />
-              },
-            })
-          ).displayName
-        ).toBe('Connect(Bar)')
-
-        expect(
-          connect((state) => state)(
-            // eslint: In this case, we don't want to specify a displayName because we're testing what
-            // happens when one isn't defined.
-            /* eslint-disable react/display-name */
-            createClass({
-              render() {
-                return <div />
-              },
-            })
-            /* eslint-enable react/display-name */
-          ).displayName
-        ).toBe('Connect(Component)')
       })
 
       it('should expose the wrapped component as WrappedComponent', () => {
@@ -2923,7 +2899,13 @@ describe('React', () => {
           </React.StrictMode>
         )
 
-        expect(spy).not.toHaveBeenCalled()
+        if (IS_REACT_18) {
+          expect(spy).not.toHaveBeenCalled()
+        } else {
+          expect(spy.mock.calls[0]?.[0]).toEqual(
+            expect.stringContaining('was not wrapped in act')
+          )
+        }
       })
     })
 
diff --git a/test/components/hooks.spec.tsx b/test/components/hooks.spec.tsx
@@ -7,6 +7,8 @@ import * as rtl from '@testing-library/react'
 import '@testing-library/jest-dom/extend-expect'
 import type { AnyAction } from 'redux'
 
+const IS_REACT_18 = React.version.startsWith('18')
+
 describe('React', () => {
   describe('connect', () => {
     afterEach(() => rtl.cleanup())
@@ -146,9 +148,9 @@ describe('React', () => {
       expect(mapStateSpy2).toHaveBeenCalledTimes(3)
 
       // 2. Batched update from nested subscriber / C1 re-render
-      // expect(renderSpy2).toHaveBeenCalledTimes(2)
-      // TODO Getting 3 instead of 2
-      expect(renderSpy2).toHaveBeenCalledTimes(3)
+      // Not sure why the differences across versions here
+      const numFinalRenders = IS_REACT_18 ? 3 : 2
+      expect(renderSpy2).toHaveBeenCalledTimes(numFinalRenders)
     })
   })
 })
diff --git a/test/hooks/useSelector.spec.tsx b/test/hooks/useSelector.spec.tsx
@@ -1,6 +1,7 @@
 /*eslint-disable react/prop-types*/
 
 import React, { useCallback, useReducer, useLayoutEffect } from 'react'
+import ReactDOM from 'react-dom'
 import { createStore } from 'redux'
 import * as rtl from '@testing-library/react'
 import {
@@ -10,9 +11,14 @@ import {
   connect,
   createSelectorHook,
 } from '../../src/index'
+import type {
+  TypedUseSelectorHook,
+  ReactReduxContextValue,
+} from '../../src/index'
 import type { FunctionComponent, DispatchWithoutAction, ReactNode } from 'react'
 import type { Store, AnyAction } from 'redux'
-import type { TypedUseSelectorHook, ReactReduxContextValue } from '../../src/'
+
+const IS_REACT_18 = React.version.startsWith('18')
 
 describe('React', () => {
   describe('hooks', () => {
@@ -498,7 +504,13 @@ describe('React', () => {
             </ProviderMock>
           )
 
-          expect(() => normalStore.dispatch({ type: '' })).not.toThrowError()
+          const doDispatch = () => normalStore.dispatch({ type: '' })
+          // Seems to be an alteration in behavior - not sure if 17/18, or shim/built-in
+          if (IS_REACT_18) {
+            expect(doDispatch).not.toThrowError()
+          } else {
+            expect(doDispatch).toThrowError()
+          }
 
           spy.mockRestore()
         })
diff --git a/yarn.lock b/yarn.lock
