when items are removed from chained lists/refs, events fire twice

[michael-brade]

I am so happy that derby is moving forward again! Thanks so much :-)

I have hit a bug in racer that is too complex for me to fix without investing a lot of time, and it turns out that there is a unit test for it that is skipped: correctly dereferences chained lists/refs when items are removed. Apparently, I am the only one hitting this because I have never heard anyone complain about it. Is there any chance it will get fixed medium-term?

Until then, I would also be very happy with a temporary hack that makes racer behave correctly. If you can think of such hack and let me know the idea, I'll implement it.

[ericyhwang]

Nate doesn't appear to have seen this yet, so I took a brief look.

That test results in paint getting two items removed, instead of the intended one.

---- starting state ----
choices [ { id: 'nature', colors: [ 'green', 'blue', 'white' ] },
  { id: 'flag', colors: [ 'red', 'white', 'blue' ] } ]
choice (choices.0) { id: 'nature', colors: [ 'green', 'blue', 'white' ] }
paint [ { id: 'green', rgb: [ 0, 255, 0 ], hex: '#0f0' },
  { id: 'blue', rgb: [ 0, 0, 255 ], hex: '#00f' },
  { id: 'white', rgb: [ 255, 255, 255 ], hex: '#fff' } ]

---- after remove ----
choices [ { id: 'nature', colors: [ 'green', 'white' ] },
  { id: 'flag', colors: [ 'red', 'white', 'blue' ] } ]
choice (choices.0) { id: 'nature', colors: [ 'green', 'white' ] }
paint [ { id: 'green', rgb: [ 0, 255, 0 ], hex: '#0f0' } ]

The refList code works by adding a model listener, and the listener has some code that I think tries to prevent the listener from reacting to itself: https://github.com/derbyjs/racer/blob/5606aae781740ee7da85b383661664b47c6cf49f/lib/Model/refList.js#L20

Given that check and the fact that chained refs are getting too many things removed, I suspected that there might be a case where that de-duping fails due to an intermediate ref, e.g. A -> B -> C -> A.

I hacked together some quick stack-tracking code in RefList#onMutation, and there indeed a cycle. It appears to be A -> B -> A:

If I change the debugger; statement in that screenshot into a return;, the test does pass and produce the correct resulting data.

• However, keeping a stack at the module level or even at the model level feels error-prone.
• I'd also worry about short-circuiting too much. Perhaps there's a scenario where some refList change triggers a change in a different part of the same refList.

Probably going to need @nateps to help brainstorm potential solutions here, if he's got the time.

[michael-brade]

Wow, this is a good even if temporary idea! I will try in December and see if this works for my app or if there are any corner cases that break with short-circuiting.

[michael-brade]

Hm. I don't know what to say... I fixed and updated my libraries and my app, and I know the racer bug is still there... but now my app works flawlessly - even without short-circuiting! :-/ For now I cannot help in finding corner cases, but I will as soon as I hit this bug again.

[craigbeck]

Thanks for your contributing your issue.

We have recently been actively updating the Derby and Racer packages, and both repos are now in Typescript and published with types on npm. As we have quite a few old issues open we have made the decision to close out of date issues.

If this issue still matters to you we encourage reproducing against the current versions of the repo and opening a new issue.