JuliaGizmos / Escher.jl

Composable Web UIs in Julia
https://juliagizmos.github.io/Escher.jl
Other
335 stars 63 forks source link

[WIP] Attempting to fix nested signals #191

Open izaid opened 7 years ago

izaid commented 7 years ago

This is a PR that attempts to fix the issue of broken nested signals, see https://github.com/shashi/Escher.jl/issues/190. Here is what I have found so far:

1) The Patchwork diff generated in start_updates comes from comparing against Escher.empty. Check out https://github.com/shashi/Escher.jl/blob/master/src/cli/serve.jl#L119. This is not right, the right thing to do is compare against the last rendered tile. I've made this work in the PR with a cache of sorts, and using that gives the correct Patchwork diff.

2) With 1) in place, I'm still having issues on the JavaScript side with not finding the right node to apply the patch to. Not sure why this is the case, but it may be related to 3).

3) I'm not sure if this is a problem or not, but a new nested signal appears to be created in each iteration of the outer signal. This means the signal ID constantly changes. While Patchwork patches this appropriately, it seems inefficient and means we can't memoize on signal IDs.

shashi commented 7 years ago

1) The Patchwork diff generated in start_updates comes from comparing against Escher.empty. Check out https://github.com/shashi/Escher.jl/blob/master/src/cli/serve.jl#L119. This is not right, the right thing to do is compare against the last rendered tile

Escher.empty is the initial value rendered (<div></div>) on the browser as well. So the first update should be a patch against that, right?

3) is right. nested maps are inefficient, it's great if you have figured out a way to make it efficient.

I'll go over this in more detail soon. Thanks!

izaid commented 7 years ago

Yeah, Esher.empty has to be the initial value. But the way it is currently set up, we're actually diffing against that in subsequent updates for the inner signals (which is the basic problem).

With regard to 3), where is the new Signal being constructed? I tried to hunt this down, but failed.