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

TypeScript error in 4.1: Type instantiation is excessively deep and possibly infinite #534

Closed
Wedvich opened this issue Oct 29, 2021 · 12 comments · Fixed by #537
Closed

Comments

@Wedvich
Copy link

Wedvich commented Oct 29, 2021

After upgrading to 4.1, we're seeing the following TS error among many of our selectors:

Type instantiation is excessively deep and possibly infinite.

We rely heavily on selector composition in our app so some of these are deeply nested (as in, their input selectors have input selectors that have input selectors etc.) - 7-8 levels deep for most of the offending ones. I haven't been able to find specific repro steps, but it doesn't seem to happen with simple/more shallow selectors.

Are there any constraints/limitations with the new types? Are there any new best practices in how we define our selectors that can help avoid this? All createSelector use inferred types (we don't define the type parameters manually anywhere).

@markerikson
Copy link
Contributor

Yeah, I did see this pop up on another project.

Can you please provide a repo or CodeSandbox that demonstrates this happening, so we can look into it further?

@Wedvich
Copy link
Author

Wedvich commented Oct 29, 2021

I was able to repro it here: https://github.com/Wedvich/reselect-4.1-ts-repro/blob/main/selectors.ts#L14

selector9 in the above repo should produce the error. So it looks like it happens when the nesting gets too deep, regardless of the complexity.

@eXamadeus
Copy link
Contributor

eXamadeus commented Nov 1, 2021

Hmm, this is a (somewhat) common problem with recursive (often conditional) types in TypeScript. See microsoft/TypeScript#30188 for a detailed discussion on the error.

Long story short, there is an intrinsic limitation to the number of recursions we can do inside our library types. Because the selectors are using the same types internally, it's triggering this mechanism inside TypeScript.

I was able to do some minor adjustments to the types and get to 15 nested selectors, which is better than the present 10 selector limit; however, it's not a very good solution long term. I will likely make a PR for that change since it cleans up some of the types, but that's not relevant here.

What WILL work, however, is simply typing some of your intermediate selectors. For example, change your example to the following, you will see no typing errors:

  type State = { foo: string }
  const readOne = (state: State) => state.foo

  const selector0 = createSelector(readOne, one => one)
  const selector1 = createSelector(selector0, s => s)
  const selector2 = createSelector(selector1, s => s)
  const selector3 = createSelector(selector2, s => s)
  const selector4 = createSelector(selector3, s => s)
  const selector5 = createSelector(selector4, s => s)
  const selector6 = createSelector(selector5, s => s)
  const selector7 = createSelector(selector6, s => s)
  const selector8: Selector<State, string> = createSelector(selector7, s => s)
  const selector9 = createSelector(selector8, s => s)
  const selector10 = createSelector(selector9, s => s)
  const selector11 = createSelector(selector10, s => s)
  const selector12 = createSelector(selector11, s => s)
  const selector13 = createSelector(selector12, s => s)
  const selector14 = createSelector(selector13, s => s)
  const selector15 = createSelector(selector14, s => s)
  const selector16 = createSelector(selector15, s => s)
  const selector17: Selector<State, string> = createSelector(selector16, s => s)
  const selector18 = createSelector(selector17, s => s)
  const selector19 = createSelector(selector18, s => s)
  const selector20 = createSelector(selector19, s => s)
  const selector21 = createSelector(selector20, s => s)
  const selector22 = createSelector(selector21, s => s)
  const selector23 = createSelector(selector22, s => s)
  const selector24 = createSelector(selector23, s => s)
  const selector25 = createSelector(selector24, s => s)
  const selector26: Selector<State, string> = createSelector(selector25, s => s)
  const selector27 = createSelector(selector26, s => s)
  const selector28 = createSelector(selector27, s => s)
  const selector29 = createSelector(selector28, s => s)
  // ...

Is it ideal? No, not really. But it's a limitation typescript has on recursive types. It's not necessarily a bad practice to type some of the intermediary selectors, anyway. Sanity checks can sometimes be helpful.

You can actually get fancy with the intermediate types, if you wish:

  // ...
  const selector17: Selector<
    State,
    ReturnType<typeof selector16>
  > = createSelector(selector16, s => s)
  const selector18 = createSelector(selector17, s => s)
  // ...
  const selector25 = createSelector(selector24, s => s)
  const selector26: Selector<
    // this can be created as a utility type, or we could expose this from the
    // library since it already exists...
    typeof selector25 extends Selector<infer S> ? S : never,
    ReturnType<typeof selector25>
  > = createSelector(selector25, s => s)

In the above examples, I use the "previous" selectors to figure out parts of the intermediate type. For selector26 we actually infer the entire intermediate type from the previous selector, but we use a weird conditional type typeof selector25 extends Selector<infer S> ? S : never,. This is actually a utility type alias we use internally in reselect called GetStateFromSelector whose implementation looks like so:

type GetStateFromSelector<S> = S extends Selector<infer State> ? State : never

You can just copy that into your project and use the pattern of selector26 to create intermediate types like so:

const initialSelector = createSelector(...)
const nestedSelector: Selector<
  GetStateFromSelector<typeof initialSelector>,
  ReturnType<typeof initialSelector>
> = createSelector(...)

@markerikson
Copy link
Contributor

/cc @JoshuaKGoldberg

@markerikson
Copy link
Contributor

Out in https://github.com/reduxjs/reselect/releases/tag/v4.1.2 , should be an improvement

@pkyeck
Copy link

pkyeck commented Nov 17, 2021

I have the same problem but with multiple selectors I pass around...
How could I go about this and get rid of the warnings?

// EpisodeTagSelectors
//
const getById = (state: RootState) => state.episodeTags.byId;

//
//
export const getEpisodeTagByIdFunc = createSelector(
  getById,
  getTagByIdFunc,
  //
  (byId, $getTagById) => (id: number) => {
    const episodeTag = byId[id];
    if (episodeTag == null) {
      return null;
    }
    return {
      ...episodeTag,
      tag: $getTagById(episodeTag.tag),
    };
  }
);

//
//
export const getEpisodeTagsByIdsFunc = createSelector(
  getEpisodeTagByIdFunc,
  //
  ($getEpisodeTagById) => (ids: Array<number>) => {
    return ids.map((id) => $getEpisodeTagById(id));
  }
);

// ....

// EpisodeSelector
//
const getById = (state: RootState) => state.episodes.byId;
const getAllIds = (state: RootState) => state.episodes.allIds;

//
//
export const getEpisodeByIdFunc = createSelector(
  getById,
  getEpisodeLocalizedByIdsFunc,
  getResourceByIdFunc,
  getEpisodeTagsByIdsFunc,
  getEpisodeVersionRequirementsByIdsFunc,
  //
  (
      byId,
      $getEpisodeLocalizedByIds,
      $getResourceById,
      $getEpisodeTagsByIds,
      $getEpisodeVersionRequirementsByIds
    ) =>
    (id: number) => {
      const episode = byId[id];
      if (episode == null) {
        return null;
      }

      return {
        ...episode,
        localizedEpisodes: $getEpisodeLocalizedByIds(episode.localizedEpisodes),
        thumbnail: $getResourceById(episode.thumbnail),
        tags: $getEpisodeTagsByIds(episode.tags),
        requirements: $getEpisodeVersionRequirementsByIds(episode.requirements),
      };
    }
);

//
//
export const getEpisodeById = createSelector(
  getEpisodeByIdFunc,
  (_: RootState, id: number) => id,
  //
  ($getEpisodeById, id) => $getEpisodeById(id)
);

@markerikson
Copy link
Contributor

@pkyeck : how many levels of selector nesting are you seeing this with?

Per the earlier comment in this thread (which is now linked from the FAQ in the README), the suggestion is to break up the chain of inference by explicitly using the Selector or OutputSelector types as a variable declaration.

@pkyeck
Copy link

pkyeck commented Nov 17, 2021

@markerikson thanks for the quick reply.

how many levels of selector nesting are you seeing this with?

Hard to say, didn't count so far ... maybe 7-8. but in multiple directions.
getEpisodeById uses 3, but inside the getEpisodeByIdFunc I also query for getEpisodeLocalizedByIdsFunc which has another 6 levels or so.

For the simple selectors, it would look like this:

const getById: Selector<RootState, EpisodeByIdState> = (state) => state.episodes.byId;

But what about the more complex ones like this one? Could you maybe point me in the right direction ...

export const getEpisodeByIdFunc = createSelector(
  getById,
  getEpisodeLocalizedByIdsFunc,
  getResourceByIdFunc,
  getEpisodeTagsByIdsFunc,
  getEpisodeVersionRequirementsByIdsFunc,
  //
  (
      byId,
      $getEpisodeLocalizedByIds,
      $getResourceById,
      $getEpisodeTagsByIds,
      $getEpisodeVersionRequirementsByIds
    ) =>
    (id: number) => {
      const episode = byId[id];
      if (episode == null) {
        return null;
      }

      return {
        ...episode,
        localizedEpisodes: $getEpisodeLocalizedByIds(episode.localizedEpisodes),
        thumbnail: $getResourceById(episode.thumbnail),
        tags: $getEpisodeTagsByIds(episode.tags),
        requirements: $getEpisodeVersionRequirementsByIds(episode.requirements),
      };
    }
);

Any help is much appreciated!

@markerikson
Copy link
Contributor

markerikson commented Nov 17, 2021

The Selector type takes three generics:

  • State: the first arg to all of the input selectors
  • Result the final return value of the generated selector
  • Params: the type representing all additional args to the input selectors

So, you need to capture the args and return value that are actually being used here. If all of these input selectors only take State as their one and only argument, then Selector<RootState, number> is all you need. If they also take an additional couple params, then you might need Selector<RootState, MyReturnType, [number, string, boolean]>, etc

@pkyeck
Copy link

pkyeck commented Nov 17, 2021

All of the nested selectors just need the state.
The first one getById returns a value directly but all the others return a function that requires one parameter.

type test = ReturnType<typeof getEpisodeLocalizedByIdsFunc>;
// equals
type test = (ids: number[]) => EpisodeLocalized[];

Do I then need the Params? As I understand it, no.

But this still gives me an error:

export const getEpisodeByIdFunc: Selector<
  RootState,
  (id: number) => Episode
> = createSelector(
  getById,
  getEpisodeLocalizedByIdsFunc,
  getResourceByIdFunc,
  getEpisodeTagsByIdsFunc,
  getEpisodeVersionRequirementsByIdsFunc,
  // ...

@markerikson
Copy link
Contributor

markerikson commented Nov 17, 2021

That's not the right type signature. The second type is just the return type of the final generated selector function. So, Selector<RootState, Episode> should work.

In other words, the Selector type describes "what would this signature look like if it was a plain function by itself?":

const mySelector = (state: State, ...params: Params) => Result

@pkyeck
Copy link

pkyeck commented Nov 17, 2021

Ahhh, got it. My mistake. It was the correct type signature but the problem was, that with the destructuring in the return type and everything else that was going on there the object did not exactly match type Episode. So I had to go fix the other selectors and this is now working:

//
//
export const getEpisodeByIdFunc: Selector<RootState, (id: number) => Episode> = createSelector(
  getById,
  getEpisodeLocalizedByIdsFunc,
  getResourceByIdFunc,
  getEpisodeTagsByIdsFunc,
  getEpisodeVersionRequirementsByIdsFunc,
  //
  (
      byId,
      $getEpisodeLocalizedByIds,
      $getResourceById,
      $getEpisodeTagsByIds,
      $getEpisodeVersionRequirementsByIds
    ) =>
    (id: number) => {
      const episodeNormalized = byId[id];
      if (episodeNormalized == null) {
        return null;
      }

      return {
        ...episodeNormalized,
        localizedEpisodes: $getEpisodeLocalizedByIds(episodeNormalized.localizedEpisodes),
        thumbnail:
          episodeNormalized.thumbnail != null
            ? $getResourceById(episodeNormalized.thumbnail)
            : null,
        tags: $getEpisodeTagsByIds(episodeNormalized.tags),
        requirements: $getEpisodeVersionRequirementsByIds(episodeNormalized.requirements),
      };
    }
);

Thanks for helping me out!

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 a pull request may close this issue.

4 participants