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

Type error when trying to use spread syntax to pass matchers to isAnyOf or isAllOf #2209

Closed
crhistianramirez opened this issue Apr 6, 2022 · 5 comments
Labels
help wanted Extra attention is needed typescript

Comments

@crhistianramirez
Copy link

crhistianramirez commented Apr 6, 2022

I have a big list of thunks that I want to observe in order to power loading indicators in a pseudo-global way and accomplishing this with matchers.

Instead of writing this which is tedious and error-prone:

const interestingThunks = [
   thunk1,
   thunk2
];
const isLoading = isAnyOf(thunk1.pending, thunk2.pending);
const isNotLoading = isAnyOf(thunk1.fulfilled, thunk2.fulfilled, thunk1.rejected, thunk2.rejected);

...

builder.addMatcher(isLoading, (state) => {
  state.loading= true;
});
builder.addMatcher(isNotLoading , (state) => {
  state.loading= false;
});

I would prefer to write this:

const interestingThunks = [
   thunk1,
   thunk2
];
const interestingPendingThunks = interestingThunks.map(thunk => thunk.pending);
const interestingFulfilledThunks = interestingThunks.map(thunk => thunk.fulfilled);
const isLoading = isAnyOf(...interestingPendingThunks );
const isNotLoading = isAnyOf(...interestingFulfilledThunks );

...

builder.addMatcher(isLoading, (state) => {
  state.loading= true;
});
builder.addMatcher(isNotLoading , (state) => {
  state.loading= false;
});

But I get the following type error when I try that:

A spread argument must either have a tuple type or be passed to a rest parameter.ts(2556)

Which seems to be happening because the type definition is:

export declare function isAnyOf<Matchers extends [Matcher<any>, ...Matcher<any>[]]>(...matchers: Matchers): (action: any) => action is ActionFromMatcher<Matchers[number]>;

There is a rest argument, but the first argument is a matcher. The workaround is to provide the first argument as normal to satisfy the condition and use the rest syntax for the others. This is a bit more verbose and not intuitive. My guess is that the first argument is defined as such so that it isn't possible to try and call isAnyOf without any arguments but maybe there is a way to have our cake and eat it as well.

Workaround:

const interestingThunks = [
   thunk1,
   thunk2
];
const interestingPendingThunks = interestingThunks.map(thunk => thunk.pending);
const interestingFulfilledThunks = interestingThunks.map(thunk => thunk.fulfilled);
const isLoading = isAnyOf(interestingPendingThunks[0], ...interestingPendingThunks );
const isNotLoading = isAnyOf(interestingFulfilledThunks[0], ...interestingFulfilledThunks );

...

builder.addMatcher(isLoading, (state) => {
  state.loading= true;
});
builder.addMatcher(isNotLoading , (state) => {
  state.loading= false;
});
@markerikson
Copy link
Collaborator

Seems worth tweaking, although I don't have time to look into this myself atm.

@markerikson
Copy link
Collaborator

It'd still be nice to get this tweaked, but it's low priority. Dropping it out of the 1.9 milestone. If anyone wants to tackle it, please let us know!

@markerikson markerikson removed this from the 1.9 milestone Aug 16, 2022
@markerikson markerikson added help wanted Extra attention is needed typescript labels Aug 16, 2022
@Faithfinder
Copy link

I'll repost the gist of my opinion here from Twitter, just in case: so, the problem is, current type signature intentionally requires one element, however .map doesn't guarantee you that the result will have one (e.g if none are pending, the array is empty).

So the question is really, whether isAnyOf with no arguments is valid or not. If it IS, then the types just need to make the constraint simplified to <Matchers extends [ ...Matcher<any>[]]>. If not, however, then the types work exactly as intended.

@crhistianramirez
Copy link
Author

That makes sense. My vote is to make isAnyOf and isAllOf accept zero arguments then. Its an easy check and would fix this issue and make the types easier to read in my opinion.

@markerikson
Copy link
Collaborator

Merged, will be out in 1.9.2 shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed typescript
Projects
None yet
Development

No branches or pull requests

3 participants