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

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

Open
wdoug opened this issue Aug 14, 2020 · 6 comments
Open

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

wdoug opened this issue Aug 14, 2020 · 6 comments
Labels

Comments

@wdoug
Copy link
Contributor

wdoug commented Aug 14, 2020

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.

@pke pke added the bug label Aug 15, 2020
@pke
Copy link
Owner

pke commented Aug 16, 2020

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

@wdoug
Copy link
Contributor Author

wdoug commented Aug 16, 2020

🤔 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
Copy link
Contributor Author

wdoug commented Aug 16, 2020

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).

@pke
Copy link
Owner

pke commented Aug 17, 2020

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
Copy link
Owner

pke commented Aug 17, 2020

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?

@wdoug
Copy link
Contributor Author

wdoug commented Aug 25, 2020

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. 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants