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 parameter expansion and improve OutputSelector readability #552

Merged
merged 7 commits into from Nov 17, 2021

Conversation

markerikson
Copy link
Contributor

@markerikson markerikson commented Nov 17, 2021

This PR:

  • Changes the behavior of MergeParameters to only do a single level of type expansion rather than recursive. This fixes [RTKQ] Type conflicts with TypedUseSelectorHook when using reselect 4.1.3 redux-toolkit#1750 , where RTKQ types broke any selector definitions, but retains some improvement of intersected parameters (intersected objects will look like {a, b, c} rather than {a} & {b} & {c})
  • Updates the OutputSelectorFields type to infer the Result type as ReturnType<Combiner>, and removes the Result generic. This simplifies the preview type content for a selector considerably.
  • Modifies MergeParameters to replace empty object parameters with never, under the assumption that this is the result of two objects with incompatible fields being intersected (such as {a: number} & {a: string}). This will force compile errors if you attempt to use them. Deeper nested incompat should be caught by TS directly.
  • Actually fixes the // @ts-ignore hack I'd thrown into the middle of createStructuredSelector, because that doesn't work when you compile the types. Still not sure why a tuple can't be spread there, but laundering it as state: Head<SelectorParams>, ...params: Tail<SelectorParams> is a legit fix now.

This gives us better-looking intersectioned objects, but avoids the
errors I saw with recursively-expanded types.
The `Result` type always comes from the `Combiner` anyway, so
removing that generic arg lets us avoid a bunch of extra types
showing up in hover previews.
@codecov
Copy link

codecov bot commented Nov 17, 2021

Codecov Report

Merging #552 (7ed20fc) into master (e1a6822) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #552   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines          105       105           
  Branches        24        24           
=========================================
  Hits           105       105           
Impacted Files Coverage Δ
src/index.ts 100.00% <ø> (ø)

@markerikson markerikson merged commit 42b6381 into master Nov 17, 2021
@markerikson markerikson deleted the feature/413-ts-checks branch November 17, 2021 04:36
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.

[RTKQ] Type conflicts with TypedUseSelectorHook when using reselect 4.1.3
1 participant