eslint-plugin-redux-saga icon indicating copy to clipboard operation
eslint-plugin-redux-saga copied to clipboard

Add yield-effects rule support for using `all` with a variable

Open wdoug opened this issue 3 years ago • 6 comments

Following up on #71

Currently, the yield-effects rule complains about this code:

const requests = endpoints.map(endpoint => call(fetch, endpoint));
yield all(requests);

We can get around the rule by inlining the code inside the yield all() call:

yield all(endpoints.map(endpoint => call(fetch, endpoint)));

It would be lovely, however, (especially for more complicated code) if this rule supported the first use case so that we could keep the semantic value of a variable that describes what we are yielding. Obviously there is no limit to how complicated this code could get and still be valid (for example, in the above code the requests could be defined in a different function) and keeping track of that in eslint itself would be impossible without a static type system, but I'm curious if it could support this one use case.

wdoug avatar Aug 14 '20 22:08 wdoug

So basically the rule would have to check if the effect is a) used inside a map/reduce(?) function callback b) the result of the map call is never yielded

pke avatar Aug 16 '20 00:08 pke

🤔 I suppose one option would be to just ignore cases where a redux-saga effect is returned from a non-generator function and just say that we don't know where the result will be used so it is outside the scope of this lint rule. I'm not sure if that is an ideal resolution but it would avoid some false positives this lint rule could find for valid code.

wdoug avatar Aug 16 '20 04:08 wdoug

For someone that is using TypeScript and the @typescript-eslint configuration, I wonder if you could use the type information to more thoroughly test for proper usage (as this note suggests might be possible).

wdoug avatar Aug 16 '20 04:08 wdoug

I suppose one option would be to just ignore cases where a redux-saga effect is returned from a non-generator function

This would be certainly the easiest approach, lets go with this first.

pke avatar Aug 17 '20 01:08 pke

For someone that is using TypeScript and the @typescript-eslint configuration, I wonder if you could use the type information to more thoroughly test for proper usage (as this note suggests might be possible).

Which type information would help us in this specific code situation?

pke avatar Aug 17 '20 01:08 pke

I'm not sure if this is actually possible, or even if it was possible if it would be worthwhile, but in theory, you might be able to use the type information to track down all the uses of a particular redux-saga effect and either verify that they are either yielded directly or end up inside an all call that is yielded. 🤷

wdoug avatar Aug 25 '20 22:08 wdoug