search

kefirjs / kefir

A Reactive Programming library for JavaScript
https://kefirjs.github.io/kefir/
MIT License
1.87k stars 97 forks source link

Dispatcher::dispatch throws if dispatch was called after cleanup #195

Closed jeron-diovis closed 8 years ago

jeron-diovis commented 8 years ago

https://github.com/rpominov/kefir/blob/master/src%2Fdispatcher.js#L48-L53

if (this._items === null) is checked after accessing this._items.length.

rpominov commented 8 years ago

Do you have a case where that happens? Or maybe you use Dispatcher API directly? If so, you shouldn't :)

jeron-diovis commented 8 years ago

I don't) Already found that this happened because of improperly configured setup/teardown hooks in my test environment. But, I thought that it's still a logical error, so decided to report.

jeron-diovis commented 8 years ago

@rpominov Unfortunately, I need to get back to this issue.

I still can't provide a good standalone example to reproduce it, but if you really need a proof, you can take a look at this. Run npm test to ensure that it works, then uncomment this and see what happens.

I'm almost sure that the real reason it still somewhere in my code. But after spending over 10 hours in debugging, I didn't found ANY stable dependencies. Tests breaks totally randomly, and the only common thing is undefined is not an object (evaluating 'items.length') in /kefir/dist/kefir.js:218:42. So I ask you, please, either tell me what am I doing wrong, or provide that little little patch from topic.

rpominov commented 8 years ago

Sorry for the trouble @jeron-diovis , submit a PR?

jeron-diovis commented 8 years ago

@rpominov ok, here it is: https://github.com/rpominov/kefir/pull/197 I didn't add any tests because have no idea how to actually reproduce my issue.

rpominov commented 8 years ago

I took a closer look, there is actually no problem you described in the first comment. We don't read this._items.length but items.length instead, which points to the this._items at the moment dispatch was called.

So the problem is this._items equals null at the moment when dispatch is called. Which is not right, and by fixing it in this place we'll probably only hide the real problem. I'll try to look at your code, hopefully later today.

jeron-diovis commented 8 years ago

Ok, thank you.

jeron-diovis commented 8 years ago

Well... Right now I did run Karma with Chrome instead of Phantom, and problem just gone. Without any changes in code. Probably, issue can be closed then? I'm so sorry if it took your time.

rpominov commented 8 years ago

Alright, feel free to close and reopen if problem comes back.