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

[sompylasar]

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

[pke]

Yes, interesting hint. Now I have to find out how to check for that.

[pke]

fixed in 7c811c27 You can re-open if its not working as you hoped it would.

[sompylasar]

@pke Sorry, please reopen because I can't, there's no button. See my comment here why your solution isn't right: https://github.com/pke/eslint-plugin-redux-saga/commit/7c811c27607f8284caf758965a270cb78c7f839a#commitcomment-18338350

[pke]

Thanks, I knew something is wrong there :) I'll add your proposed solution

[pke]

So what kind of rule are you suggesting here? This plugin should warn if the user overrides/hides a saga-effect?

[sompylasar]

I suggest the plugin should ensure that the yield operator operates on the effect creator return value, the effect creator being imported from the redux-saga module in the files where redux-saga is imported.

[pke]

Understood. I'll implement that then :)

[pke]

@sompylasar what do you think of PR #4?

[sompylasar]

@pke PR #4 is good, left some comments there. Would this new rule replace the original yield-effects one? Looks like they are the same to me.

[pke]

Added some more changes to #4 and docs.

Could it replace the yield-effects rule? That would mean the rule would then have to first check if an effect is overridden, and secondly if an effect is yielded.

[sompylasar]

The purpose of the yield-effects rule, as I see it, is to ensure that the imported effect creators' return values are yielded. This can be acomplished with one complex rule or a set of simpler rules.

Example test cases (some of them are truly marginal):

import { take } from 'redux-saga';
const sagaTake = take;
export default function* saga() {
  yield sagaTake();
}

import { take } from 'redux-saga';
export default function* saga() {
  const sagaTake = take;
  yield sagaTake();
}

import { take } from 'redux-saga';
export default function* saga() {
  const sagaTakeEffect = take();
  yield sagaTakeEffect;
}

import { take as sagaTake } from 'redux-saga';
export default function* saga() {
  yield sagaTake();
}

import { take as sagaTake } from 'redux-saga';
const sagaTakeCreator = sagaTake;
export default function* saga() {
  yield sagaTakeCreator();
}

import { take as sagaTake } from 'redux-saga';
export default function* saga() {
  const sagaTakeCreator = sagaTake;
  yield sagaTakeCreator();
}

import { take as sagaTake } from 'redux-saga';
export default function* saga() {
  const sagaTakeEffect = sagaTake();
  yield sagaTakeEffect;
}

import { fork } from 'redux-saga';
export default function* saga() {
  yield [ fork(), fork(), fork() ];
}

import { fork } from 'redux-saga';
export default function* saga() {
  const forkEffects = [ fork(), fork(), fork() ];
  yield forkEffects;
}

The ideal implementation which won't miss any case should likely start from finding all the references to effect creators imported from redux-saga, then find usages of these, ensure they are called as a function somewhere later, and ensure the return value of that function gets yielded.

Simpler implementation could assume that there is no re-assignment of effects to other variables (only directly imported ones) and re-use of the created effects (assignment of return value of effect creator to another variable). But, for example, fork effects' main usage pattern is to yield an array of them, so it would be not as easy to verify the array has been yielded.

[pke]

Is this really a case of valid code? Wouldn't the take() be called instantly and defeat the purpose of the following yield? If it is valid code, then the current lint rule check fails to detect it being good code.

import { take } from 'redux-saga';
export default function* saga() {
  const sagaTakeEffect = take();
  yield sagaTakeEffect;
}

[sompylasar]

Yes, the take function, as well as other fork, put etc., is just an effect creator which returns a plain object describing the effect. The object is then yield'ed to the redux-saga engine (it's similar to a plain object action in redux where it is yielded to the redux engine via dispatch function). The redux-saga engine handles the yield'ed value and adds it to the queue of effects to be executed.

[pke]

Ok sure, that makes sense. However I am unsure of how to rewrite the rule to catch this case as valid. Maybe you can give it a try?

[sompylasar]

I could, but for now everything I can afford is to consult and give advice... Sorry.

[burabure]

I think #11 fixes this