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

Inlining wrappers over createSelector causes selector args to be of any type #559

Open
BlackFoks opened this issue Nov 30, 2021 · 10 comments

Comments

@BlackFoks
Copy link

When upgrading from 4.0.0 to 4.1.5, our selectors started to complain about any types when we inline our createSelector wrapper:

  • TS7006: Parameter 'featureA' implicitly has an 'any' type.
  • TS7006: Parameter 'featureB' implicitly has an 'any' type.

TypeScript version is 4.4.2.

CodeSandbox example

Here's a code sandbox example of the issue (line 66):
https://codesandbox.io/s/reselect-typescript-issues-xsgeq?file=/src/App.tsx

Description

Code example:

const selectComposedFlagInline = createSelector(
  createPreferenceSelector("featureA"),
  createPreferenceSelector("featureB"),
  (featureA, featureB) => featureA && featureB === "release"
  // ^^^^      ^^^^   Parameters implicitly has an 'any' type.
);

Here createPreferenceSelector is our helper to build typed preference selectors. It's included into code sandbox example:

function createPreferenceSelector<P extends PrefName>(prefName: P) {
  return (state: AppState) => selectPreferenceValue(state, prefName);
}

Extracting preference selector into its own variable solves the issue:

const selectFeatureA = createPreferenceSelector("featureA");
const selectFeatureB = createPreferenceSelector("featureB");

const selectComposedFlag = createSelector(
  selectFeatureA,
  selectFeatureB,
  (featureA, featureB) => featureA && featureB === "release"
);

Removing the helper entirely also solves the issue:

const selectComposedFlagNoHelper = createSelector(
  (state: AppState) => selectPreferenceValue(state, "featureA"),
  (state: AppState) => selectPreferenceValue(state, "featureB"),
  (featureA, featureB) => featureA && featureB === "release"
);

While it didn't look like a huge issue, it made me very curious about why inlining caused that issue. For me it looks like all three examples should work just fine.

After some testing it looks like this issue was introduced in 4.1.0.

@markerikson
Copy link
Contributor

Huh. I'll be honest, I have no idea what's going on here and am not really even sure how to start debugging this one :)

(This goes along with my Twitter comment the other day that "what we really need is a types-level debugger that would tell us how TS is analyzing and transforming types at each step in the process".)

Given that there seem to be workarounds and that I've got other stuff going on, I'm afraid this one is going to be relatively low priority, and I'm doubtful at first glance that I'd even be able to figure out what's happening or what a solution is.

I may toss it out on Twitter or something and see if anyone else has a clue what's going on here.

@BlackFoks
Copy link
Author

Thanks for the response! It's not urgent at all, the workarounds work fine for now.

what we really need is a types-level debugger that would tell us how TS is analyzing and transforming types at each step in the process

It would be awesome to see that.

@markerikson
Copy link
Contributor

Yeah. If you look at the commit/issue history of this repo over the last few weeks, you can see that I (and a few others I've dragged in) have been iterating on the types repeatedly. Every time we fix one issue, something else pops up. I feel pretty good about the types in 4.1.5, tbh - they pass all the typetest scenarios we've got (and I've added a bunch of additional scenarios based on these issues), and based on the thought and design we've put into how they work, they seem to be doing what they should logically in combining parameters.

That process would have been much easier if I'd had better ways to inspect the intermediate steps of the types, though :)

@garrettm
Copy link

garrettm commented Dec 1, 2021

Potentially the same issue also bites me on the other end — in the composed selector calculation, here's a small sample:

interface Foo<A> {
  foo: A
}

// some selector
function foos(s: RootState) {
  return [] as Foo<string>[]
}

// extract loses type information when used with createSelector
function extract<A>(items: Foo<A>[]) {
  return items[0].foo
}

const foosDerived = createSelector(foos, extract)

// This type is unknown when it should be a Foo
const shouldBeFooButIsUnknown = foosDerived(null as any)

I believe we see this with both TS 4.4.2 and 4.5.2, hit it when upgrading from reselect 3.0.0 to 4.1.5

@markerikson
Copy link
Contributor

@garrettm what is the inferred type of foosDerived when you mouse over it, on both of the lines that it's used?

Also, what happens if you call foosDerived() with a meaningful type argument instead of as any?

@garrettm
Copy link

garrettm commented Dec 2, 2021

@markerikson here's a more full repro:

import {createSelector} from 'reselect'

interface Foo<A = unknown> {
  bar: A
}

function values<A>(_: {[k: string]: A}): A[] {
  const result: A[] = []
  const ks = Object.keys(_)
  for (const key of ks) {
    result.push(_[key])
  }
  return result
}

interface State {
  foos: {[k: string]: Foo<string>}
}

const state: State = {foos: {}}
const foos = (s: State) => s.foos

const foosList = createSelector(foos, values)
const thisShouldBeFoos: Foo[] = foosList(state)

const foosListWorking = createSelector(foos, f => values(f))
const thisIsFoos: Foo[] = foosListWorking(state)

in this example, I see this error:

typescript$ tsc inference.ts --target es2017 --moduleResolution node
inference.ts:25:7 - error TS2322: Type 'unknown[]' is not assignable to type 'Foo<unknown>[]'.
  Property 'bar' is missing in type '{}' but required in type 'Foo<unknown>'.

25 const thisShouldBeFoos: Foo[] = foosList(state)
         ~~~~~~~~~~~~~~~~

  inference.ts:4:3
    4   bar: A
        ~~~
    'bar' is declared here.


Found 1 error.

Even though both calls are identical, there's just an extra function call in the working one. I see unknown[] as the type of thisShouldBeFoos in this example.

@markerikson
Copy link
Contributor

@garrettm fwiw, while you're definitely seeing something, it looks to be a very different problem than what was originally described in this issue.

Pasting that example into the TS playground, the type of foosList as it's being used is:

const foosList: (state: {
    foos: {
        [k: string]: Foo<string>;
    };
}) => unknown[]

So, somehow the final return type is the problem here, and I'm not sure where it's going wrong in the middle.

@doarrison
Copy link

Think this is the result of using ReturnType<> directly on a function with generics (ReturnType is used in ExtractReturnType) which returns unknown. As far as I know this is a limitation on generics in typescript and the workarounds on this page are the best one can do at the moment.

@markerikson
Copy link
Contributor

Huh. That... sounds very plausible, yeah.

@aryaemami59
Copy link
Contributor

I don't think this is an issue anymore as the types were simplified and tested prior to v5.

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

5 participants