pke/eslint-plugin-redux-saga

ESLint rules for redux-saga

eslint-plugin-redux-saga's Issues

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

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.

Allow `return` of effects using `yield-effects` rule?

Sometimes it's necessary and idiomatic to directly return the result of an effect. Using the yield-effects linter rule in this case motivates the usage of return yield effect() syntax which feels odd.

For generators in general, both yield and return expressions result in values being returned. Both are legit outputs. Sandwiching the extra yield inside there doesn't actually appear to be necessary. There's no reason to pause the generator before ending it. You can just end it. So return yield "works", but it seems strange for a linter to insist on. 🤔

This maybe doesn't come up for people because most generators probably primarily deal with yields. Note that return yield could have specialized uses in non-saga generators: https://stackoverflow.com/a/42309429

If I'm wrong about this I would be grateful for any clarification.

Installation

I am facing an issue with [email protected] and vs code. I get this error
Failed to plugin redux-saga: Cannot find module 'eslint-plugin-redux-saga'. Although it is installed.

yield-effects doesn't support destructuring

the following syntax triggers a false positive on yield-effects

const [foo, bar] = yield [
  call(PrintersAPI.fetchPrinters, optsWithCapabilities),
  call(PrintersAPI.fetchManualAddedPrinters, optsWithCapabilities),
  ]

I'll submit a fix during the weekend

no-yield-in-race handle arrays

When passing an array of effects as show in the API, no-yield-in-race doesn't properly handle it and errors with: race must have on object hash as the only argument

yield-effects - delegate yield* expression

Following code...
yield* takeLatest(LOGIN, funcLogin);

...displays a warning...
takeLatest effect must be yielded

which is confusing when trying to achieve blocking execution.

ESLint suggests to fix this with following
yield* yield takeLatest(LOGIN, funcLogin);
which doesn't look correct to me and is non blocking.

Disable triggering in tests

While testing, I compare the output from each yield of the generator with put, call... functions in the test. But this is obviously triggering the rule:

expect(
  generator.next().value
).toEqual(
  put({ ... })
);

Has anyone else come across this issue? For now I'm disabling the rule on saga test files, but maybe something could be changed?

Enforce usage of selector functions on state in sagas

For the same reasons than https://github.com/pgilad/eslint-plugin-react-redux/blob/master/docs/rules/use-selectors-on-state.md, we should enforce usage of selector functions on state in sagas.

No unhandled errors error thrown when using `takeLeading`

Version: v1.1.1
Related issues: #40

Please let me know if I'm using takeLeading incorrectly, but I don't believe I am. With the following code, an error is thrown with the message:

A Saga must handle its effects' errors (use try/catch)eslint(redux-saga/no-unhandled-errors)

function* rootSaga() {
  try {
    yield all([
      takeEvery(Action.foo, saga1),
      // Only this line throws
      takeLeading(Action.bar, saga2),
      takeLatest(Action.baz, saga3),
    ]);
  } catch (e) {
    // ...
  }

Document rules

I can't find an overview of what the specific rules are, it'd be nice to add that to the README!

Can't install the 0.1.1 version

I do not have any problem with Internet connection

Unnecessary strict dependency?

Is it necessary to strictly bind to eslint version 3.19.0? Would be nice to get rid of that in favour of a more generic version as suggested in the npm docs. This way the following warning could be prevented:

npm WARN [email protected] requires a peer of eslint@^3.19.0 but none was installed.

eslint-plugin-redux-saga/package.json

Line 33 in 41c49c3

"eslint": "^4.1.0",

catch `put(function*)`

A rule should prevent you from using put with a function generator as first argument.

Rule proposal: Ensure yield on call to generator function

As proposed, but rejected due that Generator functions deemed a niche use-case: typescript-eslint/typescript-eslint#7749
And apparently proposed before: typescript-eslint/typescript-eslint#1068

Description

My suggested rule would check that every call to a generator, inside another generator is called with yield

Fail Cases

function* parentGenerator(){
  console.log("parentGenerator:: start");
  childGeneratorOne();
  yield* childGeneratorTwo();
  console.log("parentGenerator:: end");
}

function* childGeneratorOne(){
  console.log("childGeneratorOne:: start");
}

function* childGeneratorTwo(){
  console.log("childGeneratorTwo:: start");
}

const itr = parentGenerator();
console.log(itr.next());

Pass Cases

function* parentGenerator(){
  console.log("parentGenerator:: start");
  yield* childGeneratorOne();
  yield* childGeneratorTwo();
  console.log("parentGenerator:: end");
}

function* childGeneratorOne(){
  console.log("childGeneratorOne:: start");
}

function* childGeneratorTwo(){
  console.log("childGeneratorTwo:: start");
}

const itr = parentGenerator();
console.log(itr.next());

Additional Info

In the failing case, the childGeneratorOne is never called, which you won't expect if you miss that yield is lacking...

Was suggested before eslint/eslint#5193 but, as described in the issue, type info is needed to make this work.

Provide support for eslint 8

Would be great to have this package support eslint 8.

no-unhandled-error expects try-catch around effects that can't be caught

Saga doc, in the Error Propagation section, points out "you can't catch errors from forked tasks".
Using the no-unhandled-error rule, I still need to put effects such as fork and takeEvery (which is also forked) in catch blocks, even though they don't do anything. This also means I can never achieve code-coverage on those catch blocks.
Maybe there could be exception for specific effects.

Please update to latest version of redux-saga

With latest version we have error: "Error: Cannot find module 'redux-saga/lib/effects'", because effects now should be imported from 'redux-saga/effects';

The `yield-effects` rule should check if the found functions are imported from `redux-saga/effects`

Currently it just compares the name, but arbitrary codebase may have functions named put, spawn, etc. and this will give false positives.

yield-effects autofix modifies code functionality violating eslint best practices

I just tried to add this plugin to our codebase and noticed that if with code like this:

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

The autofix for the yield-effects rule will update code to be:

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

There are two less than ideal things here, so I'll open a second issue for the second request, but the first is that the runtime functionality of the code is changed. The eslint best practices for applying fixes say this:

Avoid any fixes that could change the runtime behavior of code and cause it to stop working

As of eslint 6.7 (released Nov 2019), there is a new suggestions API that allows editors to provide an optional ability for users to apply fixes that could change the underlying code functionality.

Peer dependency warning

Expected behavior

No peer warn

Actual behavior

npm WARN [email protected] requires a peer of redux-saga@>= 0.11.1 < 1 || 1.0.0-beta.3 but none is installed. You must install peer dependencies yourself.

Steps to reproduce

• npm install --save [email protected]

Environment

Tech	Version
redux-saga	1.0.0
eslint-plugin-redux-saga	0.10.0

Allow effects to be used without yield in arrays

Using this model to construct a root saga gives linting errors since the takeEvery isn't yield'ed

redux-saga/redux-saga#160 (comment)

Release notes

Can you write release notes (eg via the Github releases page) for us to understand the changes made in between releases ?
I'd really appreciate if you could follow semantic releases, thanks in advance!

Error in redux-saga/yield-effects

For plugin v0.2.0
and code like this:

import { put } from 'redux-saga/effects';

function* exampleSaga() {
  // ... some code
  yield [
        put(someActionCreator()),
        put(anotherActionCreator()),
  ];
}

get following linting error:
error put effect must be yielded redux-saga/yield-effects

404 page

I go on https://www.npmjs.com/package/eslint-plugin-redux-saga and click on no-yield-in-race and I got 404

Update deps version: redux-saga

[email protected] requires a peer of redux-saga@^0.11.1

Latest redux-saga version is 0.12

Version 10 of node.js has been released

Version 10 of Node.js (code name Dubnium) has been released! 🎊

To see what happens to your code in Node.js 10, Greenkeeper has created a branch with the following changes:

• Added the new Node.js version to your .travis.yml

If you’re interested in upgrading this repo to Node.js 10, you can open a PR with these changes. Please note that this issue is just intended as a friendly reminder and the PR as a possible starting point for getting your code running on Node.js 10.

More information on this issue

Greenkeeper has checked the engines key in any package.json file, the .nvmrc file, and the .travis.yml file, if present.

• engines was only updated if it defined a single version, not a range.
• .nvmrc was updated to Node.js 10
• .travis.yml was only changed if there was a root-level node_js that didn’t already include Node.js 10, such as node or lts/*. In this case, the new version was appended to the list. We didn’t touch job or matrix configurations because these tend to be quite specific and complex, and it’s difficult to infer what the intentions were.

For many simpler .travis.yml configurations, this PR should suffice as-is, but depending on what you’re doing it may require additional work or may not be applicable at all. We’re also aware that you may have good reasons to not update to Node.js 10, which is why this was sent as an issue and not a pull request. Feel free to delete it without comment, I’m a humble robot and won’t feel rejected 🤖

---

FAQ and help

There is a collection of frequently asked questions. If those don’t help, you can always ask the humans behind Greenkeeper.

---

Your Greenkeeper Bot 🌴

yield-effects rule triggers when it shouldn't

I've just tried implementing this in react-boilerplate, however, the yield-effects rule is triggering on this line, which seems like a bug to me?

Cannot perform object spread inside of a race object

Given

import { race, take } from 'redux-saga/effects'

function* saga() {
    const waitEffects = {
        success: take(SomeSuccessAction),
        failure: take(SomeFailureAction),
    }
    
    yield race({
        ...waitEffects,
        other: take(otherAction),
    })
}

Running eslint with the above results in:

TypeError: Cannot read property 'name' of undefined
Occurred while linting /Users/noah/test-saga.js:9
at checkYieldInObject (/Users/noah/node_modules/eslint-plugin-redux-saga/lib/rules/no-yield-in-race.js:11:29)
at CallExpression (/Users/noah/node_modules/eslint-plugin-redux-saga/lib/rules/no-yield-in-race.js:81:13)
at /Users/noah/node_modules/eslint/lib/linter/safe-emitter.js:45:58

This happens in the "no yield in race" rule. In checkYieldInObject, it assumes each property has a "key". The ExperimentalSpreadProperty node does not have a key.

If it's not reasonable to lookup the referenced object, a warning should be reported that object spread is not supported.

Publish package

Can't find it under eslint-plugin-redux-saga on npm, am I missing something?

False positive for rule no-yield-in-race when Array.map() is used inside race()

The following code triggers the no-yield-in-race rule ("race must have on object hash or array as the only argument") even though the argument to race() is an array and thus valid.

function* foo() {
  const actions = ["LOGIN", "LOGOUT"]
  yield race(actions.map(take))
}