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

The select operator and data inheritance. #22

Closed
mbostock opened this issue Nov 5, 2010 · 2 comments · May be fixed by Houssk/d3#1
Closed

The select operator and data inheritance. #22

mbostock opened this issue Nov 5, 2010 · 2 comments · May be fixed by Houssk/d3#1
Labels
bug Something isn’t working

Comments

@mbostock
Copy link
Member

mbostock commented Nov 5, 2010

Usually, when the select operator is run, each newly-selected node inherits its data from the corresponding node in the original selection. However, this was recently changed so that if the new selection already hd data, it would not inherit data from the original:

if (subnode && !subnode.__data__) subnode.__data__ = node.__data__;

However, I'm not sure this is what we want.

First, what if the data is an array of numbers or booleans (or null, rather than undefined)? This could be very confusing in terms of some of the newly-selected nodes inheriting data and some of them not.

Second, the above implementation prevents the new selection from inheriting the data, even if the original selection has data defined. Perhaps we should check if the data was set explicitly on the original selection, so that it can override what's stored in the DOM's __data__?

@mbostock
Copy link
Member Author

Fixed. Changed to

if (subnode && "__data__" in node) subnode.__data__ = node.__data__;

@saifelse
Copy link

I think this is still problematic. The code snippet found here: https://github.com/mbostock/d3/wiki/Selections#wiki-data has nested data. In trying to call select on a single td, its data (a single number) gets overwritten by its parent td's data (an array of numbers). My workaround for now, has been to do selectAll and then filter to reduce the size of the selection to the element I care about. The issue is demonstrated in this fiddle: http://jsfiddle.net/FUJa6/1/

I feel as though the fix is more confusing than the original implementation. It is also strange that select modifies __data__ , while selectAll does not.

Fix 1
It seems like in most cases, if a child node already has data, you would not want it to inherit its parent's data. To fix the boolean/null problem, we could do:

if (subnode && !("__data__" in subnode)) subnode.__data__ = node.__data__;

Fix 2
A second alternative would be to consider the 'freshest' data; whichever data is newest should win. __data__ could be augmented with a global counter that tracks when the data was set. If both the subnode and node have data, the data with a higher counter would win. This seems overly complex, and I can't think of a good example of when this scenario arises.

Fix 3
To get rid of any ambiguities, I think it is cleaner to never inherit data from the parent. select is then non-volatile. In the cases that you do want to use the parent data, you can use each, to create a closure. For example, instead of:

d3.selectAll('div').data(divData);
d3.selectAll('div').selectAll('p').text(function(d){ return d;});

you would do:

d3.selectAll('div').data(divData);
d3.selectAll('div').each(function(divData){
    d3.select(this).selectAll('p').text(divData);
});

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn’t working
Development

Successfully merging a pull request may close this issue.

2 participants