RFC: Consistency and Control

[hshoff]

Breaking changes are painful. I try to avoid them whenever possible. Getting vx to v1 means I need to make a larger breaking change than I am typically comfortable with as a library author. In short, I made a poor design decision in the early days of this library that I now wish to fix. I hope you'll agree with me that it's for the best and will lead to a brighter future with more consistency and control.

Summary

This pull request does the following:

• removes two utility functions used by vx components: additionalProps() and callOrValue()
• adds the more common react pattern of render props to components impacted by the removal of the above two utils.
• removes scale props in favor accessors that return scaled values
• renames zScale prop to color
• adds more PropTypes
• adds more tests
• update all vx-demo.now.sh/gallery tiles to new api

Motivation

Almost 2 years ago I started working on a library of components that brought together react and d3 at a lower-level than traditional charting libraries. Unfortunately, I made an api design choice that I've regretted for a long time now and it is time to rectify before releasing v1.

Data Binding Before

Initially I thought the target audience would be existing d3 developers that found themselves in a react codebase. So I went about making an api that would feel similar to d3 when it came to data binding.

Let's walk through an example of rendering out a series of bars that are different colors based on each datum's name. (data = [{ name: 'captain gary' }, // ...])

// d3
svg.append("g")
 .selectAll("rect")
 .data(data)
 .enter().append("rect")
  .attr("fill", datum => color(datum.name))

So one coffee-fueled morning I thought "fine I'll bind props to data in react if you pass a function as a prop"

// vx
<Bars
  data={data}
  fill={datum => color(datum.name)}
/>

But wait, what about props that are typically functions like event handlers? Say you wanted to click on each colored bar and alert the bar's name. Well you would just pass a function that returns your click handler with the datum as the first argument.

// vx
<Bars
  data={data}
  fill={datum => color(d.name)}
  onClick={datum => event => {
   alert(d.name);
  }}
/>

🤦‍♂️ 🤦‍♂️

What's so bad about this? It no longer feels like react. vx components become special in the sense you need to know about this silly pattern and it's not like other react components. This is confusing: #50 #199 #328

Data Binding After

Then came the realization that the target audience for vx was really react developers that needed data visualization without having to learn d3.

Seems like a confusing api tradeoff for the added ability of setting a prop based on a datum value. Along came render props. Let's take a look at what the new api looks like with render props.

<Bars
  data={data}
  // ...
>
   {bars => {
    return bars.map((bar, i) => {
      return (
        <rect
          key={bar.name}
          fill={color(bar.name)}
          onClick={event => alert(bar.name)}
        />
      );
     })
   }}
</Bars>

This means vx components are no longer opaque boxes with special rules. You now have more control over what's rendered and it's more transparent how it works following common patterns in react land.

This is a change I've wanted to make for a long time. It took a lot boring work to make it happen, but I think it's finally ready.

What about Hooks?

I was just as surprised as everyone else about the react Hooks announcement at ReactConf. The day they were announced, I was finishing migrating all of the gallery tiles to the new api and I had the panic moment of, "oh no this new api is going to be dead-on-arrival".

The Hooks announcement talked about how hooks can replace render props with the benefit of not introducing a false hierarchy to your render. This is great and I look forward to updating vx to include hooks. That said, Hooks are still a proposal available on an alpha release of react. Currently, vx supports react 15 and 16. vx will look to support hooks in a future release. I see the render props api as an immediate improvement for all users of vx where hooks would only support those lucky enough to be in a codebase on 16.7 alpha.

Initial thoughts are hooks are the future of react, so vx will support them when they are more widely available.

Accessors + Scales Before

In the beginning, I found myself often having to apply my scale on the accessor value. So I turned this into a pattern in vx. You would pass in your scale and your accessor separately and vx would apply them for you. It used look like this:

 <LinePath
  data={data}
  xScale={xScale}
  yScale={yScale}
  x={x}
  y={y}
/>

behind the scenes vx would create a line path from d3-shape's line generator

const path = line()
  .x((...args) => xScale(x(...args)))
  .y((...args) => yScale(y(...args)))
  .defined(defined)
.curve(curve);

Again this isn't transparent. Let's make it transparent.

Accessors + Scales After

<LinePath
  data={data}
  x={d => xScale(x(d))}
  y={d => yScale(y(d))}
/>

This api simplifies what vx is doing behind the scenes as well to:

const path = line();
if (x) path.x(x);
if (y) path.y(y);
if (defined) path.defined(defined);
if (curve) path.curve(curve);

This gives transparency, consistency with the d3 api, and gives you more control over shape generation.

Closing thoughts

I worry about the early adopters that are not going to be happy about having to update their code due to these breaking changes, but the longer the I wait to make this the bigger the hill will be to climb.

Would love to hear your thoughts.

Changelog

💥 Breaking Changes

• [glyph][breaking] rm additionalProps, add children as fn
• [shape][breaking] rm additionalProps, add children as fn
• [geo][breaking] rm additionalProps, add children as fn
• [heatmap][breaking] rm additionalProps, add children as fn
• [stats][breaking] rm additionalProps, add children as fn
• [boxplot][breaking] rm additionalProps, add children as fn
• [voronoi][breaking] rm additionalProps, add children as fn
• [legend][breaking] rm additionalProps, add children as fn

🏠 Internal

• [demo] update gallery tile examples to new apis

cc @williaster @conglei @techniq

[techniq]

Should we consider dropping our HOCs as part of this breaking change? See: #220

[sto3psl]

But wait, what about props that are typically functions like event handlers?

I always kind of liked the datum => event => {} style. It worked well if one doesn't have to customize the rendered component that much. If I understand it correctly, with the new API, one has to always render the actual SVG and vx just provides the data. This makes a lot of sense in the low-level nature of vx.

For the repo I'm working on it isn't a big deal so go for it. I like it when things get more consistent and changes will reduce overall confusion for new users! We use vx since v0.0.158.

The scale and accessor change is really nice as well. More power to the user.

Nice job on the proposed changes and that's quite some work you had to do there 😅!