tomwanzek / d3-v4-definitelytyped

[DEPRECATED] This repo was intended as a staging area for typescript definitions supporting the latest major release of D3js (i.e. version 4.1.x) by Mike Bostock. It has been migrated to DefinitelyTyped.
MIT License
53 stars 14 forks source link

Ambiguity in `data` key function argument #54

Closed zbjornson closed 8 years ago

zbjornson commented 8 years ago

In this:

d3.select("p")
    .data([0, 1, 2], function (d) { return d.toString(); });

d is inferred as {} | number, which I think is an unnecessary ambiguity.

Changing this:

    data<NewDatum>(
        data: Array<NewDatum>,
        key?: (this: GElement | PElement, datum?: Datum | NewDatum, index?: number, group?: Array<GElement | PElement> | ArrayLike<GElement | PElement>) => string
    ): Selection<GElement, NewDatum, PElement, PDatum>;

to this:

    data<NewDatum>(
        data: Array<NewDatum>,
        key?: (this: GElement | PElement, datum?: NewDatum, index?: number, group?: Array<GElement | PElement> | ArrayLike<GElement | PElement>) => string
    ): Selection<GElement, NewDatum, PElement, PDatum>;

resolves the ambiguity, but I'm not sure if there's a use case I'm missing where the original Datum and not the NewDatum would be passed to the key fn.

tomwanzek commented 8 years ago

I'll have to look at it again tomorrow, I thought I saw something in the source code, where the key function was invoked in two different spots with slightly different arguments.

zbjornson commented 8 years ago

Ah you're right, my bad.

This key function is evaluated for each selected element, in order, being passed the current datum (d), the current index (i), and the current group (nodes), with this as the current DOM element. The key function is then also evaluated for each new datum in data, being passed the current datum (d), the current index (i), and the group’s new data, with this as the group’s parent DOM element.