d3 types don't match the version we're using

[jameshadfield]

I encountered this while fixing a specific bug in d3, but I suspect the problem is much more widespread within Auspice rather than just that described below, hence the broad issue title.

background

The d3-selection library we use to bind event listeners is version 1.4.2. Viewing past API docs for d3 libraries is difficult but the relevant docs (which match my testing) are these:

When a specified event is dispatched on a selected element, the specified listener will be evaluated for the element, being passed the current datum (d), the current index (i), and the current group (nodes), with this as the current DOM element (nodes[i]).

Because we define the listener as a fat-arrow function this is not rebound to the current DOM element and remains PhyloTree (PhyloTreeType).

types don't match reality
Changing the NodeCallback type to match reality, type NodeCallback = (this: PhyloTreeType, d: PhyloNode) => void causes a very interesting type error (we're using @types/d3-selection version 3.0.11):

src/components/tree/phyloTree/renderers.ts:146:18 - error TS2769: No overload matches this call.
  Overload 1 of 3, '(typenames: string, listener: null): Selection<SVGPathElement, PhyloNode, SVGDefsElement, unknown> & ((this: SVGPathElement, event: any, d: PhyloNode) => void) & ((this: SVGPathElement, event: any, d: PhyloNode) => void)', gave the following error.
    Argument of type 'NodeCallback' is not assignable to parameter of type 'null'.
  Overload 2 of 3, '(typenames: string, listener: (this: SVGPathElement, event: any, d: PhyloNode) => void, options?: any): Selection<SVGPathElement, PhyloNode, SVGDefsElement, unknown> & ((this: SVGPathElement, event: any, d: PhyloNode) => void) & ((this: SVGPathElement, event: any, d: PhyloNode) => void)', gave the following error.
    Argument of type 'NodeCallback' is not assignable to parameter of type '(this: SVGPathElement, event: any, d: PhyloNode) => void'.
      The 'this' types of each signature are incompatible.
        Type 'SVGPathElement' is not assignable to type 'PhyloTreeType'.

Overload 2, (typenames: string, listener: (this: SVGPathElement, event: any, d: PhyloNode) => void, options?: any), is the signature of the current version of d3-selection, not the one we're using (note the first argument is event not the datum`). Overload 1 looks similarly wrong. I don't know why tsc doesn't show me overload 3.

So it seems the type definitions we are using don't match the d3 libraries we're using.

The types seemingly forbid far-arrow functions

The type definitions (regardless of the fact that they are for a different d3 version than we're using) look like they enforce this being what d3 sets it to via listener.call(this, ...); which essentially forbids the use of fat-arrow functions (as they inherit the this in scope where the function was called from). This seems very limiting - fat-arrow functions are very useful for just this reason!

[victorlin]

Thanks for the detailed write-up. I haven't fully wrapped my head around the fat-arrow function stuff, but I've opened PR #1913 to align the type versions.

[genehack]

Thanks for the detailed write-up. I haven't fully wrapped my head around the fat-arrow function stuff

MDN has a good explanation of the "don't bind this" thing — there are times when this is actually useful, and then there are times when it turns around and bites you…

[jameshadfield]

I consider this closed by #1913. I suspect we'll run into issues as we expand our usage of TS because I deliberately used fat-arrow functions to allow this: PhyloTreeType, but when we actually run into issues we can re-open this issue or create a new one.