Improve type-safety with `data`

[Drup]

After having played a bit with d3 .. It's really easy to get exceptions by using data not exactly the way it's supposed to be used.

My understanding is that the safe idiom is :

selection
|. data (fun d i -> ... )
|- nest enter [ ... ]
|- nest update [ ... ]
|- nest exit [ ... ] 

with selection something obtained from select or selectAll (not run).

Is it right ? If that's the case, why not just hardcode it ? I propose adding the type selection returned by selecting functions and:

val with_data : 
  selection -> ('a -> int -> 'b list) -> 
  enter:('b,'b) t -> update:('b,'b) t -> exit:('b,'b) t -> ('a, 'b) t

[seliopou]

Yes, your understanding is correct that that is the safe idiom that most D3.js code follows, with two exceptions. First, an enter should always be followed by an append.

selection
|. data (fun d i -> ... )
|- nest (enter <.> append "element") [...]
|- nest update [ ... ]
|- nest exit [ ... ] 

Second, reordering of the various cases does sometimes occur when doing animations, or transitions in D3 parlance.

Your proposal is very close to what's required in order to make data binds type-safe, but not quite. Perhaps a clarification of type-safety in this context is necessary. First, note that data binds mutate the data bound to existing elements. Also, remember that sequencing cannot track changes to types of data involved in this mutation. Hence:

(selectAll "things" : (_, int) D3.t)
|- data (fun _ _ _ -> ["one"; "two"; "three"])
|- attr (fun _ d _ -> string_of_int d) (* val d : string, actually *)

... will type check, but is not type-safe.

Second, the enter selection is not a normal selection. It has an interface that is limited in comparison to other selections, as you discovered in a separate issue. This is because the elements it contains are actually pseudoelements—placeholders for future insertions into the DOM. Hence:

enter <.> str attr "class" "whoops"

... will type check, but will produce a runtime error.

The former is more what I think of as unsoundness, (e.g., absence of the value restriction) while the latter is more of an API property that isn't expressed in types (e.g., n / 0).

In order to avoid the unsoundness, it's sufficient to introduce the following operator:

val safe_data : string -> ('a -> int -> 'b list) -> ('a, 'b) t

This simply eliminates the possibility of involving the initial selectAll in a sequencing operation, which is the source of the unsoundness.

To avoid the runtime error from misuses of enter, you can again introduce an additional operator that just combines the enter and append into one step:

val safe_enter : string -> ('a, 'a) t

I was considering adding the safe_data operator because that really is an unsoundness issue that could be addressed easily by including it in a submodule. If you want unsound data, just open D3. But if you want sound data,

open D3
include Safe_bind (* : sig
  val bind : string -> ('a -> int -> 'b list) -> ('a, 'b) t
*)

I suppose a similar thing could be done with safe_enter, but to me that's less compelling. The issue I have with the labeled argument solution is that it is not entirely obvious what the ordering will be, especially given that labeled arguments can be reordered as one pleases.

[Drup]

First, an enter should always be followed by an append.

Oh, I got bit by that one later on, yeah.

The safe_enter and safe_data sounds like very good combinators, and I agree we don't necessary need the labeled version I proposed. I would however argue that the safe version should be the default (and the unsafe stuff separated into an Unsafe module). Good defaults are important, and it will cover 90% of the use cases.