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

Add imitate construct #66

Closed OlivierBlanvillain closed 7 years ago

OlivierBlanvillain commented 7 years ago

This PR adds the imitate construct, inspired from xstream's imitate (thanks @bbarker for the idea!).

This implementation doesn't require an additional impure method but adds an extra boolean field to all Var. An alternative would be to add a new subclass of Rx, ImitatingVar, without := but with imitate instead. This would increase the surface API to lower the memory footprint, probably not worth doing.

Best reviewed commit by commit :smile:

bbarker commented 7 years ago

LGTM. Tested it in a TodoMVC app with cycles.

OlivierBlanvillain commented 7 years ago

Great, could you update #53 with whatever you have on the TodoMVC? I'm going to merge this PR for now and but wait for an proper example before making another release.

OlivierBlanvillain commented 7 years ago

I just realised I completely disregarded the case where someone tries to imitate a Var twice. The current implementation will do first one and silently ignore the second one. It would be nice to use the same mechanism than in #60 to add "don't to that" warning.

bbarker commented 7 years ago

I didn't forget, but thought that could be done as a second PR. Will be happy to get to it next week!

On Aug 4, 2017 9:13 AM, "Olivier Blanvillain" notifications@github.com wrote:

I just realised I completely disregarded the case where someone tries to imitate a Var twice. The current implementation will do first one and silently ignore the second one. It would be nice to use the same mechanism than in #60 https://github.com/OlivierBlanvillain/monadic-html/pull/60 to add "don't to that" warning.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OlivierBlanvillain/monadic-html/pull/66#issuecomment-320245572, or mute the thread https://github.com/notifications/unsubscribe-auth/AA37jvQucKHU8s2DKNUexMsIS5in-JC-ks5sUxkVgaJpZM4OrXhB .