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

shallowEqual breaks typed selector return type #1964

Closed
1 task
justgowa opened this issue Oct 25, 2022 · 12 comments
Closed
1 task

shallowEqual breaks typed selector return type #1964

justgowa opened this issue Oct 25, 2022 · 12 comments

Comments

@justgowa
Copy link

What version of React, ReactDOM/React Native, Redux, and React Redux are you using?

  • React: 18.2.0
  • ReactDOM/React Native: 18.2.0
  • Redux: 4.2.0
  • React Redux: 8.0.4

What is the current behavior?

without shallowEqual type inferred correctly
image

otherwise it breaks
image

What is the expected behavior?

types infer correctly with shallowEqual

Which browser and OS are affected by this issue?

No response

Did this work in previous versions of React Redux?

  • Yes
@justgowa
Copy link
Author

#1348 found related issue

@phryneas
Copy link
Member

phryneas commented Oct 25, 2022

Could you try installing the CodeSandbox preview build from #1965 and see if that fixes it for you?

(go to https://ci.codesandbox.io/status/reduxjs/react-redux/pr/1965 and follow the install instructions)

@justgowa
Copy link
Author

justgowa commented Oct 25, 2022

Unfortunally no but useSelector still has same type
image

I mean NoInfer is not used

When i dive into useSelector.d.ts i see

export declare const useSelector: <TState = unknown, Selected = unknown>(selector: (state: TState) => Selected, equalityFn?: EqualityFn<Selected> | undefined) => Selected;

Then i tryed to add NoInfer type here and is still didn't work.


I realised I'm using TypedUseSelectorHook, then went to its type declaration and added NoInfer.

export interface TypedUseSelectorHook<TState> {
    <TSelected>(selector: (state: TState) => TSelected, equalityFn?: EqualityFn<NoInfer<TSelected>>): TSelected;
}

Voila, It's working now

@phryneas
Copy link
Member

phryneas commented Oct 25, 2022

Ah, very good catch, that one needs it too!
I updated the PR - is it working out of the box for you now?

@justgowa
Copy link
Author

justgowa commented Oct 26, 2022

Yes, thanks a lot

@justgowa
Copy link
Author

@phryneas will you release these changes?

@phryneas
Copy link
Member

We'll need a few type tests for that before @markerikson pulls the lever on that - I'll try to get to that in the next few days. Meanwhile, that CI build should work nicely I hope?

@markerikson
Copy link
Contributor

Hmm. Fwiw I just tried setting up a typetest based on this and #1348 , and it seems to be working correctly without NoInfer.

@justgowa by any chance do you have any of TS's "strict" settings turned off?

I'm okay with shipping the change because I don't think it will hurt, but after a quick try I can't repro the original issue.

@markerikson
Copy link
Contributor

Fixed by #1965 .

@justgowa
Copy link
Author

justgowa commented Nov 7, 2022

@markerikson yep, ts strict is off

@phryneas
Copy link
Member

phryneas commented Nov 7, 2022

@justgowa we cannot make any guarantees for any TypeScript configuration other than strict: true. We already support the last 5 TypeScript releases.
With all the flags that could be enabled or disabled on top of that, there would be tens of thousands of configuration combinations we have to check.
Also: TypeScript is meant to be used with strict. The fact that you can turn that off is pretty much a backwards compatibility stunt or for during a migration to TS, but you should always aim for enabling strict.

@justgowa
Copy link
Author

justgowa commented Nov 7, 2022

@phryneas i get it, i'm working on pretty old project and unfortunatly it started with non strict rules.
Anyway, thank you a lot!

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

No branches or pull requests

3 participants