Rewrite of MergeParameters in v4.1.7 breaks type inference with spreading in createSelector

[PeterBul]

Before version 4.1.7 and the rewrite of MergeParameters, it was possible to write

interface ITestState {
  foo: number;
  bar: string;
}

const selectFoo = (state: ITestState) => state.foo;
const selectBar = (state: ITestState) => state.bar;

const selectors = [selectFoo, selectBar];

export const selectFooBar = createSelector(
  [selectFoo, selectBar, ...selectors],
  (foo, bar, ...rest) => {
   ...
  }
);

This would infer the type of newSelector to be (state: ITestState) => ... & ... since all selectors used as input had that type.

In 4.1.7, after the rewrite of MergeParameters, newSelector is of type (state: never) => ...

It still works with only using

export const selectFooBar = createSelector(
  [selectFoo, selectBar],
  (foo, bar) => {
   ...
  }
);

or

export const selectFooBar = createSelector(
  [...selectors],
  (...rest) => {
   ...
  }
);

but the combination does not work any longer.

Here is the example on StackBlitz.

[markerikson]

Hmm. Can you give me an example of when you would actually want to write a selector that way, and why? I've never seen that done before.

[PeterBul]

Say you want to add a series of input selectors to an array based on some data. Then you don't know how your selectors will look, but writing it like this will enable you to get an array of results which you then can use in the new selector.

Here is a toy example of a case where it might be done.

[markerikson]

Huh. Definitely not a pattern I've ever seen in use.

To be honest this is going to be very low priority - there's a lot of other things going on atm, and I don't know if/when I'll have time to look at this. If you or anyone else has a suggested fix and can add a typetest showing the improved type works correctly, that'd be appreciated!

[PeterBul]

I can see if I can figure it out 👍

[nicolaschambrier]

There might be a more general semver issue here. We recently had changes to apply to upgrade from a 4.x to 4.1.8, and we were surprised it would happen for a minor version.

When exploring a bit we found out the signatures changed a lot through minor versions, in a breaking way for the build step:

// 4.0.0
export function createSelector<S1, R1, T>(
  selectors: [Selector<S1, R1>],
  combiner: (res1: R1) => T,
): OutputSelector<S1, T, (res1: R1) => T>;

// 4.1.8
<Selectors extends SelectorArray, Result>(
    selectors: [...Selectors],
    combiner: (...args: SelectorResultArray<Selectors>) => Result,
    options?: CreateSelectorOptions<MemoizeOptions>
  ): OutputSelector<
    Selectors,
    Result,
    GetParamsFromSelectors<Selectors>,
    ((...args: SelectorResultArray<Selectors>) => Result) &
      ReturnType<MemoizeFunction>
  >

I guess it's hard to bump major for a build-only breaking change, I have no strong opinion here, but that would at least desserve a strong warning in changelog?

[EskiMojo14]

assuming you're not passing the generics yourself (which should never be done), i'm unsure which part was breaking? your usage should be identical 🤔

with regards to the original issue, adding as const to the selector array fixes the inferrence:

interface ITestState {
  foo: number;
  bar: string;
}

const selectFoo = (state: ITestState) => state.foo;
const selectBar = (state: ITestState) => state.bar;

const selectors = [selectFoo, selectBar] as const;

export const selectFooBar = createSelector(
  [selectFoo, selectBar, ...selectors],
  (foo, bar, ...rest) => {
   ...
  }
);

[markerikson]

Yeah, while there was a significant rewrite of the types in 4.1.0, from my perspective it was not at all a breaking change. The types should have been generally syntax-compatible with how Reselect was designed to be used.