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

Improve type-safety with data #5

Open
Drup opened this issue Sep 30, 2015 · 2 comments
Open

Improve type-safety with data #5

Drup opened this issue Sep 30, 2015 · 2 comments

Comments

@Drup
Copy link
Contributor

Drup commented Sep 30, 2015

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
Copy link
Owner

seliopou commented Oct 8, 2015

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
Copy link
Contributor Author

Drup commented Oct 8, 2015

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants