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

Bug in binding in a very specific configuration #650

Closed
jccazeaux opened this issue Aug 5, 2016 · 12 comments
Closed

Bug in binding in a very specific configuration #650

jccazeaux opened this issue Aug 5, 2016 · 12 comments

Comments

@jccazeaux
Copy link
Contributor

I have a very very specific bug that occurs only when you have

  • A binder rv-if with some condition
  • A binder rv-each on an array inside the rv-if
  • Inside the rv-each display array length
    Full exemple here

The bug happens when you

  1. Add data to the array (in reality it's a ajax request that adds data)
  2. Toggle the rv-if condition to remove the element (in reality the condition is on the length of the array)
  3. Reset the array
  4. Toggle the rv-if
  5. Try to add data...won't work

To get it to work again i must toggle the rv-if twice.

@blikblum
Copy link
Contributor

blikblum commented Aug 5, 2016

I managed to find why:

  • The each binder will create a observer at scope.pages
  • Adding a item, the each binder will create a child view that will reuse the scope.pages observer because of {scope.pages | length}
  • Toggling if binder off will unbind the each bindings and will delete the scope.pages observer
  • Resetting pages will create a new non observed scope.pages property
  • When if bind toggled on the each binder will create a new observer at scope.pages
  • When each binder routine is called the child view will be unbind because collection length is different from iterated length -> this will delete the scope.pages observer and reactivity is lost

@blikblum
Copy link
Contributor

blikblum commented Aug 5, 2016

If instead of reseting the array, remove one of the item the same bug occurs

@jccazeaux
Copy link
Contributor Author

Nice, that's a good explanation. Do you see a solution?

@blikblum
Copy link
Contributor

blikblum commented Aug 5, 2016

For this specific case yes, but will be symptomatic. The root issue lays deeper.

See http://codepen.io/blikblum/pen/OXBarY

Add some items
Toggle if
Try add more items

@jccazeaux
Copy link
Contributor Author

I agree, the cause is deep, maybe in the adapter's observce/unobserve functions.
We have many examples, this will make it easier to fix and test!

@jccazeaux
Copy link
Contributor Author

The issue may be in adapter. See http://codepen.io/jccazeaux/pen/WxaPdm
When no adapter is used in expressions, there is no issue.

@blikblum
Copy link
Contributor

blikblum commented Aug 6, 2016

I already fixed in my side. Just need a test

@jccazeaux
Copy link
Contributor Author

I think i'm close
See here : https://github.com/mikeric/rivets/blob/master/src/adapter.coffee#L97
When unobserve is called on array's keypath, the mutation observers (push, slice...) are deleted regardless of the other observers.

@jccazeaux
Copy link
Contributor Author

😄
I have a test, i send a PR with it right now

jccazeaux pushed a commit to jccazeaux/rivets that referenced this issue Aug 6, 2016
@jccazeaux
Copy link
Contributor Author

My working fix is to replace

          unless callbacks.length
            delete map.callbacks[keypath]

         @unobserveMutations obj[keypath], obj[@id], keypath

with

          unless callbacks.length
            delete map.callbacks[keypath]
            @unobserveMutations obj[keypath], obj[@id], keypath

It will unobserve mutations only when there are no callbacks left. You have the same?

@blikblum
Copy link
Contributor

blikblum commented Aug 6, 2016

The same

@jccazeaux
Copy link
Contributor Author

Great, i add it to the PR

jccazeaux pushed a commit to jccazeaux/rivets that referenced this issue Aug 6, 2016
jccazeaux pushed a commit to jccazeaux/rivets that referenced this issue Aug 6, 2016
blikblum added a commit to blikblum/tinybind that referenced this issue Aug 6, 2016
blikblum added a commit to blikblum/tinybind that referenced this issue Aug 6, 2016
jccazeaux pushed a commit to jccazeaux/rivets that referenced this issue Aug 7, 2016
benadamstyles added a commit that referenced this issue Aug 7, 2016
benadamstyles added a commit that referenced this issue Aug 7, 2016
Fix callback calls. See #650. Regression since #546
jccazeaux pushed a commit to jccazeaux/rivets that referenced this issue Oct 10, 2016
benadamstyles added a commit that referenced this issue Oct 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants