search

OlivierBlanvillain / monadic-html

Tiny DOM binding library for Scala.js
https://olivierblanvillain.github.io/monadic-html/examples/
MIT License
225 stars 24 forks source link

[WIP] add Rx.sharing #82

Closed bbarker closed 6 years ago

bbarker commented 6 years ago

I like both suggestions mentioned for sharing in #70 , but since the second largely depends on the first, I thought I'd try to get the ball rolling with the first. Unfortunately, updating upstream var doesn't propagate. When run operates on Sharing, the foreach (self.impure.foreach{ x => ...) doesn't seem to execute for updated values of the upstream Rx, i.e., self. I assume this is because I've made an egregious logic error, or perhaps more specifically, sharingCancelable is cancelled before it should be.

I'm also not sure if I needed to add those three sharing members to the Rx, but it seems like they needed to be persisted to the Rx.

bbarker commented 6 years ago

@OlivierBlanvillain One thing I want to point out in the test is this line:

assert(count == 6) //FIXME: should be 4?

This seems wrong to me.

Once we get this all fixed up, rather than merging directly, I can squish commits and open a new PR.

bbarker commented 6 years ago

Oh, it appears the extra 2 come from the calls to impure.value

OlivierBlanvillain commented 6 years ago

LGTM :+1: Thanks for the PR! (No need to squash, the history makes perfect sense here :smile:)