no-effect-override rule

[pke]

This is a WIP PR to fix #1. The docs are missing.

[sompylasar]

Semicolon is invalid after a function declaration (results in an extra empty statement following the declaration).

[pke]

@sompylasar Now only the docs are missing?

[pke]

I have added all test cases you mentioned in #1 (comment) except for the fork stories.

2 good tests are failing now, maybe you can have a look if they are really valid function generator use cases.

[sompylasar]

Unnecessary empty line.

[sompylasar]

We may assume these failing cases as marginal and just really report them as errors.
There is no need in the real code to pre-create effects and then yield them later.

[sompylasar]

Here and below should be no "noyield" because the actual take effect from the redux-saga is simply unused.

[pke]

I would consider that still an override error since the take override hides the saga effect import. That its most likely a serious error. If not, the user can still disable the rule for that section of the file.

[sompylasar]

Yes, there should only be the override error, but not noyield error.

[pke]

so the same is true for all overrides_and_noyield tests? The only error that should be thrown is override?

[sompylasar]

Yes. You can't say an effect wasn't yielded because what you found is not an effect, it's something else, named the same.

[pke]

Squashed for merge. What do you think? @sompylasar

[sompylasar]

Do we allow yielding something that is not an effect or array of effects?

BTW, the test for yielding array of effects is missing.

[pke]

I think general function generator linting should be done in a separate plugin or eslint core.

I'll add the other tests for arrays

[sompylasar]

It's a valid piece of code, you can generally yield any value from a generator. The question is, do we allow that for sagas (the files where the redux-saga module is imported)?

[pke]

I would say thats for another linter plugin that specifically deals with generators. The case tested above seems very unlikely to happen in a real program.

[sompylasar]

Another plugin that would deal with generators should treat this code as valid, because it is. You can yield anything.

But, when using redux-saga, you should only yield the return values of the effect creators (or arrays of such).

So, it's this eslint-plugin-redux-saga's responsibility to limit the usage of generators to only yield redux-saga effects when a saga file is detected (by assuming files which import redux-saga as files that declare sagas, and generators inside them are sagas).

[pke]

Ok, I agree. So which additional tests / checks you suggesting to complete this rule?

[sompylasar]

Sorry for the delay.

I would test for the patterns described in the redux-saga documentation, starting from here: http://yelouafi.github.io/redux-saga/docs/basics/UsingSagaHelpers.html and here: http://yelouafi.github.io/redux-saga/docs/advanced/RunningTasksInParallel.html

Note that the effect creators are imported from redux-saga/effects, but the helpers which are to be yield*ed, not yielded, are imported from redux-saga itself:

import { call, put } from 'redux-saga/effects'
import { takeEvery } from 'redux-saga'

Note that yielding an array of calls to effect creators is also valid:

// correct, effects will get executed in parallel
const [users, repos]  = yield [
  call(fetch, '/users'),
  call(fetch, '/repos')
]

[pke]

Will be able to work on that more now.