Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrect RTKQ endpoint definition types for correct selector typings #1818

Merged
merged 3 commits into from
Dec 10, 2021

Conversation

markerikson
Copy link
Collaborator

@markerikson markerikson commented Dec 10, 2021

This PR:

  • Fixes a bug in the ReducerPath type that was extracting the wrong generic from EndpointDefinitions, causing a cascade that resulted in typed Reselect selectors getting a state: {} parameter
  • Fixes what appeared to be a bug in optimisticUpdates.test.ts and additionally works around some odd behavior with mocking baseQuery() that was resulting in extra console errors

Fixes #1817 .

As best as I can tell, the incorrect ReducerPath type was there since at least 1.6.0. I think we always had a bug here, but the use of Reselect 4.0 with its looser types must have masked it. When we updated to Reselect 4.1.x with improved type inference, we uncovered this bug.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 10, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 17f033e:

Sandbox Source
Vanilla Configuration
Vanilla Typescript Configuration
rsk-github-issues-example Configuration
reduxjs/redux-toolkit Configuration
@examples-query-react/advanced Configuration
rtk-query-selector-typing-issue Issue #1817

@github-actions
Copy link

github-actions bot commented Dec 10, 2021

size-limit report 📦

Path Size
1. entry point: @reduxjs/toolkit (cjs.production.min.js) 12.93 KB (0%)
1. entry point: @reduxjs/toolkit (esm.js) 10.79 KB (0%)
1. entry point: @reduxjs/toolkit/query (cjs.production.min.js) 22.59 KB (0%)
1. entry point: @reduxjs/toolkit/query (esm.js) 18.94 KB (0%)
1. entry point: @reduxjs/toolkit/query/react (cjs.production.min.js) 24.83 KB (0%)
1. entry point: @reduxjs/toolkit/query/react (esm.js) 21.63 KB (0%)
2. entry point: @reduxjs/toolkit (without dependencies) (cjs.production.min.js) 5.71 KB (0%)
2. entry point: @reduxjs/toolkit (without dependencies) (esm.js) 5.67 KB (0%)
2. entry point: @reduxjs/toolkit/query (without dependencies) (cjs.production.min.js) 10.04 KB (0%)
2. entry point: @reduxjs/toolkit/query (without dependencies) (esm.js) 10.47 KB (0%)
2. entry point: @reduxjs/toolkit/query/react (without dependencies) (cjs.production.min.js) 2.8 KB (0%)
2. entry point: @reduxjs/toolkit/query/react (without dependencies) (esm.js) 3.21 KB (0%)
3. createSlice (esm.js) 5.03 KB (0%)
3. createEntityAdapter (esm.js) 6.28 KB (0%)
3. configureStore (esm.js) 5.65 KB (0%)
3. createApi (esm.js) 17.18 KB (0%)
3. createApi (react) (esm.js) 19.89 KB (0%)
3. fetchBaseQuery (esm.js) 11.61 KB (0%)
3. setupListeners (esm.js) 10.42 KB (0%)
3. ApiProvider (esm.js) 18.51 KB (0%)

The `ReducerPath` type tries to extract the generic type for "the
path of the reducer" arg from `EndpointDefinitions`. However, it
looks like this was always off by one: getting the 4th arg instead
of the actual 5th arg.

This caused bad input to the generated selectors. That seems to
have worked okay with Reselect 4.0 and its looser types, but
caused problems when we updated to Reselect 4.1.x and its improved
types. The `RootState` type collapsed down to an empty object,
because there was no valid string to use as an object key.

This fixes the bug and adds a typetest to verify that the selector
types are carried through all the way.
@markerikson markerikson merged commit 0c3c41d into v1.7.0-integration Dec 10, 2021
@markerikson markerikson deleted the bugfix/1817-rtkq-selectors branch December 10, 2021 03:34
@@ -464,8 +464,9 @@ export type QueryArgFrom<D extends BaseEndpointDefinition<any, any, any>> =
export type ResultTypeFrom<D extends BaseEndpointDefinition<any, any, any>> =
D extends BaseEndpointDefinition<any, any, infer RT> ? RT : unknown

export type ReducerPathFrom<D extends EndpointDefinition<any, any, any, any>> =
D extends EndpointDefinition<any, any, any, infer RP> ? RP : unknown
export type ReducerPathFrom<
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Daaamn, good find!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants