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

Combiner function arguments have unknown type #627

Closed
yndx-madfriend opened this issue Oct 15, 2023 · 4 comments
Closed

Combiner function arguments have unknown type #627

yndx-madfriend opened this issue Oct 15, 2023 · 4 comments

Comments

@yndx-madfriend
Copy link

yndx-madfriend commented Oct 15, 2023

Hi! Sorry if this was asked before. The closest issue I found was #547 and #559, if this one is a duplicate, feel free to close it.

Since we updated from 3.x to 4.1.8, we got a lot of errors with combiner function arguments getting typed as any.

I created a small playground to reproduce the issue here:

import { createSelector } from 'reselect';

createSelector(
    (_state) => 5,
    // argument five below has type "unknown"
    (five) => ({ five })
)

createSelector(
    (_state): 5 => 5,
    // argument five below has type "unknown"
    (five) => ({ five })
)

const get5 = (_state): 5 => 5;

createSelector(
    get5,
    // argument five below has type 5
    (five) => ({ five })
)

Is this the intended behavior of reselect typings?

@markerikson
Copy link
Contributor

markerikson commented Oct 15, 2023

You must provide correctly typed arguments for all of the input functions. As of Reselect 4.1, Reselect infers everything from those arguments.

Define (_state: RootState) => 5 (and similarly for any other arguments), and it will all work correctly!

@yndx-madfriend
Copy link
Author

You must provide correctly typed arguments for all of the input functions. As of Reselect 4.1, Reselect infers everything from those arguments.

Define (_state: RootState) => 5 (and similarly for any other arguments), and it will all work correctly!

I do not understand why, with the new typings of createSelector, it makes such a difference (whether to have a function inlined or defined separately). Also, as I said, it all used to work in v3, and this new requirement of having separate definitions is not explicitly stated neither in changelog nor in readme.

@markerikson
Copy link
Contributor

markerikson commented Oct 15, 2023

It's not about the "inline" or "defined separately".

It's literally that you must tell Reselect what the types of the arguments are.

// ❌ FAILS - no type for `state`
createSelector(
  (state) => state.x,
  output
)

// ✅ WORKS - TS knows the type of `state`
createSelector(
  (state: RootState) => state.x,
  output
)

Reselect's types now infer everything from those arguments, and tbh I'm not sure how this ever would have worked even with the old types if you don't provide any argument types at all.

@yndx-madfriend
Copy link
Author

Thank you for clarifications @markerikson , it seems to work correctly now with declared types of arguments. 🎉

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

2 participants