Skip to content

lint: Redirect react-redux imports through our type-wrapper module#5093

Merged
chrisbobbe merged 2 commits intozulip:mainfrom
gnprice:pr-lint-react-redux-imports
Nov 2, 2021
Merged

lint: Redirect react-redux imports through our type-wrapper module#5093
chrisbobbe merged 2 commits intozulip:mainfrom
gnprice:pr-lint-react-redux-imports

Conversation

@gnprice
Copy link
Copy Markdown
Member

@gnprice gnprice commented Nov 2, 2021

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.

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.
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.
@chrisbobbe
Copy link
Copy Markdown
Contributor

Thanks, LGTM! Merged.

@gnprice gnprice deleted the pr-lint-react-redux-imports branch November 2, 2021 03:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants