From ae347fb63658902f0bbb8d0660d1696a1981ab20 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 1 Nov 2021 17:55:19 -0700 Subject: [PATCH 1/2] redux types: Re-export Provider through our type-wrapper module Now we have no direct imports of react-redux at all except the ones in this file itself. That'll make things cleaner for adding a lint rule that we don't make such imports. --- src/boot/StoreProvider.js | 2 +- src/react-redux.js | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/boot/StoreProvider.js b/src/boot/StoreProvider.js index 1b5d4a3b73f..748a627b6f3 100644 --- a/src/boot/StoreProvider.js +++ b/src/boot/StoreProvider.js @@ -1,9 +1,9 @@ /* @flow strict-local */ import React, { PureComponent } from 'react'; import type { Node } from 'react'; -import { Provider } from 'react-redux'; import { observeStore } from '../redux'; +import { Provider } from '../react-redux'; import * as logging from '../utils/logging'; import { getAccount, tryGetActiveAccountState } from '../selectors'; import store, { restore } from './store'; diff --git a/src/react-redux.js b/src/react-redux.js index 135dce0d1ec..3069de1d56b 100644 --- a/src/react-redux.js +++ b/src/react-redux.js @@ -9,6 +9,11 @@ import { import type { PerAccountState, GlobalState, Dispatch, GlobalDispatch } from './types'; import type { BoundedDiff } from './generics'; +// There's not a lot to say about the type of `Provider`, and it only has +// one use-site anyway; so we don't wrap it. But do re-export it from here, +// so everything else can uniformly never import directly from react-redux. +export { Provider } from 'react-redux'; + /* eslint-disable flowtype/generic-spacing */ // We leave this as invariant in `C` (i.e., we don't write `-C` or `+C`) From cfa22183ca7f2042970aa75094d7a5ad48b210c0 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 1 Nov 2021 17:51:07 -0700 Subject: [PATCH 2/2] lint: Redirect react-redux imports through our type-wrapper module This lets us catch bugs where, for example, a selector passed to `useSelector` doesn't return the type that the caller wants it to. Now that we have a distinction between `useSelector` and `useGlobalSelector`, it also lets us catch bugs where `useSelector` is mentioned but `useGlobalSelector` is what's actually wanted. For the moment these are both purely type-wrappers of the upstream `useSelector`, so such a bug would have no effect at runtime... but in the future a per-account state will be a different actual object from a global state, so one or both of them will not pass its selector the same state object as the upstream `useSelector` would, and then such a bug would likely cause a crash. --- .eslintrc.yaml | 5 ++++- src/react-redux.js | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/.eslintrc.yaml b/.eslintrc.yaml index 7a8674bd56c..97819071e12 100644 --- a/.eslintrc.yaml +++ b/.eslintrc.yaml @@ -227,7 +227,10 @@ rules: - devDependencies: ['**/__tests__/**/*.js', tools/**] no-restricted-imports: - error - - patterns: ['**/__tests__/**'] + - patterns: + - group: ['**/__tests__/**'] + - group: ['/react-redux'] + message: 'Use our own src/react-redux.js instead.' import/no-cycle: off # This would be nice to fix; but isn't easy. import/export: off # This is redundant with Flow, and buggy. diff --git a/src/react-redux.js b/src/react-redux.js index 3069de1d56b..0b79cf47867 100644 --- a/src/react-redux.js +++ b/src/react-redux.js @@ -1,5 +1,8 @@ /* @flow strict-local */ import type { ComponentType, ElementConfig } from 'react'; +/* eslint-disable no-restricted-imports */ +// This file is where we type-wrap items from react-redux for use in the +// rest of the codebase. import { connect as connectInner, useSelector as useSelectorInner,