refactor(filelist): rename promise -> lastCompletedRefresh and remove…

[johnjbarton]

… unused promise

[lusarz]

_isRefreshing method is not used anywhere, we can remove it.

[johnjbarton]

I was working off HEAD and forgot about your changes to this file.

[lusarz]

No problem, just wanted to mention it :)

[lusarz]

Why we need it ?

[johnjbarton]

Please read the comment below ;-)

[lusarz]

Do you think it's possible to write unit tests that fails when you replace let lastCompletedRefresh = this._refreshing with let lastCompletedRefresh - like it was before ?

[johnjbarton]

Well maybe I'm confused. To me

let lastCompletedRefresh = Promise.map(this._patterns, (patternObject) => {...})
   .then(() => { 
        if (this._refreshing !== lastCompletedRefresh) {
          return this._refreshing
        }
     })
...

means that once this statement executes, lastCompletedRefresh is set to the promise returned by the RHS. In the promise callback the value is unambiguous. However this line will cause the test to fail with ReferenceError: lastCompletedRefresh is not defined.
If we write

let lastCompletedRefresh = null
lastCompletedRefresh = Promise.map(this._patterns, (patternObject) => {...})
   .then(() => { 
        if (lastCompletedRefresh === null) {
          throw new Error('how can this be')
        }
     })
...

Then one of the unit tests throws "how can this be". I don't understand how the then call back can run before the promise assignment.

So, no, but I also don't understand the unit tests behavior now.

(This long chain of promises at different levels is to hard to read. IMO it should be refactored to make the chains clearer).

[devoto13]

Then one of the unit tests throws "how can this be". I don't understand how the then call back can run before the promise assignment.

Because of something like this, which causes promise callbacks to be executed synchronously (and therefore before the assignment).

I'm working on simplifying this piece now.