move .exit before .enter to fix removal of bubbles on redraw

[benheb]

I am having the same issue with the bubble chart as was fixed with the scatter chart in this pr: https://github.com/dc-js/dc.js/pull/1463/files. Basically when I would call redraw() on a bubble chart and pass in a new group it would update the existing bubbles, but if any were removed from the group... they would not be removed.

This pr moves the removeNodes before the update and it resolves the issue.

I did notice it doesn't seem to be an issue here: https://dc-js.github.io/dc.js/, but I tried 1000 different ways in my code base and each time the bubbles would not be removed until I reordered the .exit and .enter. My only guess is that I'm calling .redraw() on a chart and in the demo that's somehow happening internally? Not really sure.

Related issue (scatter plots) #1460

[gordonwoodhull]

Right, as I noted in #1460, this kind of bug will not be triggered in "normal" crossfilter usage, because crossfilter never removes items: the values of the bins just drop to zero.

It's only if you use a different backend, or if you use "fake groups", that this is possible.

We had a wave of these bugs earlier. Since the original library did not support these use cases, we are lacking tests and that's why it's possible for new bugs to arise.

If you would be willing to add a test where items are truly removed, that would help a lot. You could follow the pattern of the tests I added to #1463, using remove_empty_bins

[benheb]

K, will do when I have time. Added some tests just now and they are passing... just not all that sure they are actually testing what they should be testing.

[gordonwoodhull]

Cool! You should be able to revert your fixes and see the tests fail.

[benheb]

Yep, they aren't! ;-)

[gordonwoodhull]

Sounds good! Please push the tests to this branch and the PR should be ready to merge.

[gordonwoodhull]

Whoops I misread you. Right, tests that don't fail on buggy code aren't all that helpful. 😸

I'll give it a look right now. Maybe I can just copy the technique from the scatter plot tests.

[gordonwoodhull]

Okay, applying remove_empty_bins to one of the tests that was already filtering on another dimension, and then redrawing, was enough to exercise the bug.

Thanks @benheb! Merged for 3.0.6.

[benheb]

Thanks @gordonwoodhull! 👍