Silent failures with v1.1

[SkaterDad]

I'm not sure how the commits from 1.0.3 to 1.1.0 have accomplished this, but I discovered my DOM events weren't firing anymore after updating.

Most concerning is that I didn't receive any error messages.

I downgraded to 1.0.3 and everything worked normally again.

These are the observables watching for the events. Could the commit about sorting namespaces have any effect on this?

  const changeDistance$ = DOM.select('input.dist').events('input')
    .debounce(500)
    .map(ev => ev.target.value)
    .map(val => {
      return function updateDist(query) {
        *insert proprietary stuff here
      }
    })

  const changeCountry$ = DOM.select('select.country').events('change')
    .map(ev => ev.target.value)
    .map(val => {
      return function updateCountry(query) {
        *insert proprietary stuff here
      }
    })

[TylorS]

Hopefully it wouldn't, but yes it's possible. I'm just sort of curious, because there are quite a few tests for select() and events(), why none of them would fail if I accidently made a breaking change.

[TylorS]

I'll work on adding some more tests to be safe.

[SkaterDad]

I appear to have narrowed down the problem.

There seems to be an issue with the sortNamespace function on isolated elements.

The test I added in this commit fails when using that function, but passes when I comment out the .sort. It fails on the first assertion, so select() can't find anything..

test/browser/isolation.js

[TylorS]

Thank you for this. I'll add this test to my work as well. I'll work on a proper fix after work

[SkaterDad]

No problem. Just fulfilling my duties as the guy who has to break every release :stuck_out_tongue:

[TylorS]

Technically I break them, and you help me solve them :+1:

[TylorS]

I can further confirm that this is strictly related to behavior in isolate()

[TylorS]

cc @staltz

This should probably be taken into consideration with https://github.com/cyclejs/cycle-dom/issues/96 I'll PR whatever I figure out over here back to @cycle/dom

[TylorS]

@SkaterDad I think I have gathered a proper fix for this. If you don't mind looking at the tests I added before I merge and release the fix, that would be great.

[TylorS]

@staltz If you have any input on my solution that would be great too. I'll PR this same thing to cycle-dom soon.

[staltz]

Thanks @TylorS for the tests and the fix. I think so far we have been supporting only classnames. I think it's important we start moving efforts to cycle-dom mainly, because it's being rewritten in Snabbdom (it's basically a merge of this repo with cycle-dom), and naturally this repo would be discontinued.

[SkaterDad]

The tests look pretty good.

it's also great to hear the snabbdom will be used by cycle/dom going forward.

[TylorS]

Should be good with v1.1.1. Feel free to reopen if you still encounter issues @SkaterDad