Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cacheData in a BindingSeq should be re-calculated when re-watching the BindingSeq #56

Closed
Atry opened this issue Jul 28, 2017 · 7 comments
Labels

Comments

@Atry
Copy link
Collaborator

Atry commented Jul 28, 2017

Curious if I'm doing something wrong in this example code: https://scalafiddle.io/sf/HzKl1uG/4

If the checkbox is off, and I change the number, then when I click checkbox on, it doesn't reflect the number I've typed. But if dates are displayed, then changing the number has the proper effect.

Reported by @kahliburke on Gitter:
https://gitter.im/ThoughtWorksInc/Binding.scala?at=597958a6329651f46ec7752c

@Atry Atry changed the title Cache in BindingSeq should be re-calculate when re-watching the BindingSeq Cache in BindingSeq should be re-calculated when re-watching the BindingSeq Jul 28, 2017
@Atry Atry added the bug label Jul 28, 2017
@Atry
Copy link
Collaborator Author

Atry commented Jul 28, 2017

There should be a re-calculation of cacheData between line 888 and line 889

@Atry Atry changed the title Cache in BindingSeq should be re-calculated when re-watching the BindingSeq cacheData in a BindingSeq should be re-calculated when re-watching the BindingSeq Jul 28, 2017
@Atry
Copy link
Collaborator Author

Atry commented Jul 28, 2017

Another example written by @kahliburke : https://scalafiddle.io/sf/HX9dhMN/3

@Atry
Copy link
Collaborator Author

Atry commented Jul 28, 2017

Since the cacheData should be re-calculated when watching, maybe it should be cleared when unwatching, in case of memory leaks.
https://github.com/ThoughtWorksInc/Binding.scala/blob/9513541/Binding/src/main/scala/com/thoughtworks/binding/Binding.scala#L878-L879

@Atry
Copy link
Collaborator Author

Atry commented Jul 28, 2017

Also we can remove the initialization of cacheData as long as it will be computed when watching.

https://github.com/ThoughtWorksInc/Binding.scala/blob/9513541/Binding/src/main/scala/com/thoughtworks/binding/Binding.scala#L820-L824

@kahliburke
Copy link
Contributor

kahliburke commented Aug 9, 2017

I have tried some of the recommended changes which have not resolved the issue so far. While debugging it seemed that the use of a conditional was part of the issue, i.e.

@dom val elemDisplay = {
    if (show.bind)
      <div>{elems.bind}</div>
    else <!-- Nothing -->
  }

will show the issue while

@dom val elemDisplay = {
    val e = elems.bind
    if (show.bind)
      <div>{e}</div>
    else <!-- Nothing -->
  }

seems to work properly, with no other changes.

One thing I did notice that seems like a bug is that in FlatMapBinding.removePatchedListener there is a mistaken call for adding the patched listener to the children instead of removing it.

child.addPatchedListener(childListener)

@Atry
Copy link
Collaborator Author

Atry commented Aug 10, 2017 via email

@kahliburke
Copy link
Contributor

There already is one. :)

@Atry Atry closed this as completed in 8bc7845 Nov 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants