RFC: Proposal for new xy-chart package

[williaster]

Motivation

vx packages are (by design) low-level and modular which maximizes their flexibility, but also requires more work to create even simple charts. My library @data-ui/xy-chart is built on top of vx and was designed to make it easier to create common charts with less work. To solve this use-case within the vx ecosystem, and consolidate these efforts, @data-ui/xy-chart is being deprecated and we plan to port its functionality to a new vx package @vx/xy-chart.

Goals of this RFC

• align on near- and mid-term features
• align on an XYChart API
• align on a general direction for implementation

Features

To have feature parity with @data-ui/xychart, the new package should support the following:

• Computes and provides x- and y- scales across all data series
• Handles mouse events for the XYChart across all data series
  • Positions tooltips and provides tooltip data
  • Supports programmatic tooltip control
  • Mouse events can be defined at chart or series level, can be disabled at series level
• Supports the following *Series which are mostly wrappers around vx
  • Bar, Line, Point, Area, AreaDifference, StackedArea, StackedBar, GroupedBar, BoxPlot, ViolinPlot, Interval
• Exports CrossHair component for use with Tooltip
• Exports XAxis and YAxis components from vx
• Supports horizontal and vertical reference lines
• Supports x- and y-gridlines
• Supports brush functionality (pre @vx/brush)
• Supports styles with LinearGradient, Patterns, and a chart theme via props
• Wraps individual points in FocusBlur handlers that are a11y accessible / tab-able

We would also like to add additional support for the following:

New near-term features

• re-written in TypeScript
• responsive by default
• supports arbitrary datum shape + x / y data accessors (currently requires { x, y } datum shape)
• first-class hooks support

New mid-term features

• optionally render Tooltip in a Portal to fix z-index stacking context problem
• easy creation of Legends
• better support for overlays / annotations (points + regions)
• integration with @vx primitives like brush, zoom, and drag
• canvas support – vx is currently mostly svg based (this likely requires updates in other vx packages)
• animation – @data-ui does not support animation. while this may not need to be fully baked in we should expose hooks to animate xy-chart

API

@techniq has done some great research on declarative react chart APIs here. Generally they are composable:

<Chart {...}>
  <Axis {...} /> 
  <Gridlines {...} /> 
  <Legend />
  <DataSeries {...} /> 
  <DataSeries {...} />
</Chart>

However there are some key differences:

data provided at the DataSeries level vs the chart container level

1. DataSeries level – ✅ favored

() => <Chart><DataSeries data={...} /></Chart>

Pros 👍

• it's more "natural" to directly link data to the series that will visually represent it
• @vx/shape's (the basis for DataSeries) currently require data, so this is more consistent with separate package APIs
• Series can use any custom logic they need for computing the x- and y- extent from their data, and the Chart container can simply collect these
  • additionally this allows more advanced functionality like horizontal orientation to be pushed to the Series-level without requiring it to be implemented by all Series and the Chart container doesn't need to have any knowledge of it

Cons 👎

• Chart container needs a way to access the data across all series

2. Chart container level – ❌ disfavored

() => <Chart data={...}><DataSeries /></Chart>

Pros 👍

• ultimately the Chart needs access to all data in order to provide x- and y- scales; this makes that easy.

Cons 👎

• Series may require custom logic to compute x- and y- extent from their data (e.g., a bar stack) which the Chart needs to be aware of in this model
• still requires key accessors at the Series-level for Series data
• forces all DataSeries to have the same data length (or be filled with empty values)

Mouse and Touch events

Mouse and touch events are handled in several ways

1. Standard mouse events – ✅ favored
   react-vis and @data-ui expose mouse and touch events at both the Chart and Series level; these use standard react events such as onClick, onTouchMove, etc.

2. Custom event syntax – ❌ disfavored
   Some libraries like Victory have their own custom event system with non-standard syntax and selection language.

react hooks

I've not been able to find any react vis libraries which expose hooks. Feels like an opportunity on top of a render / component API 😏

Implementation

We'd like to improve upon the following limitations of @data-ui v1 implementation:

1. Written in TypeScript
   @data-ui was written in JavaScript, but vx is now a TypeScript project and typings will be similarly useful for @data-ui.

2. Use react context over cloning children
   @data-ui was implemented before the new / more robust 16.3 context API. Therefore chart styles and shared scales are passed via props + React.cloneElement. Combined with hooks, using context should open up a whole new set of API possibilities (see below).

What is kept in context

The function of the XYChart wrapper largely equates to managing shared state across the elements of a chart, which components can leverage as needed. . This includes

• chart theme + styles
• xScale that accounts for data range of all chart series and chart width + margin
• yScale that accounts for data range of all chart series and chart height + margin
• tooltipData + tooltipCoords, when applicable

In addition to chart width + height, the Chart container must have knowledge of all data (+ annotation) values to compute scales appropriately. Rather than having the Chart introspect props from child DataSeries or Annotations (which can get gnarly) we propose that DataSeries and Annotations register their data, xValues, and yValues in context.

This pushes the logic of data <> x/y extent mapping to DataSeries rather than the Chart, and allows the Chart to leverage these values to compute scales properly. It could look something like

// in e.g., <LineSeries {...props} />
const { key, data, xAccessor, yAccessor } = props;
const { registerData } = useContext(XYChart);

registerData({
  dataKey: key,
  xValues: data.map(d => xAccessor(d)),
  yValues: data.map(d => yAccessor(d)),
  // other interesting things to do at this level
  mouseEvents: false,
  legendItemRenderer,
}); 

Unknowns

I'm unsure if there are major performance implications of using hooks ⚡ 🐌

Proposed API

// all of these items have access to the same Chart `context` which includes
// theme, dataRegistry, xScale, yScale, colorScale, tooltipData, tooltipCoords
const { ChartProvider, XYChart, Legend, Tooltip } = useChart({ theme, scaleConfig, ... }));

() => (
  {/* context provider */}
  <ChartProvider>
   {/** 
     * Chart renders `svg` container and computes `x-` and `y-scale`s using the 
     * data registry context. It is either passed `width`/`height`, or it uses 
     * `@vx/responsive` for auto-sizing
     */}
    <XYChart>
     {/** 
       * DataSeries register their data on mount. When `x-` and `y-scale`s 
       * are computed and available in context they render data. 
       */}
      <LineSeries key={dataRegistryKey} data={...} />
    
     {/** 
       * Axes use `x-` or `y-scale`s from context based on orientation.
       */}
      <Axis orientation="left" />

      {/* Custom axis component could use `scale`s from context */}
      <CustomAxis />
    </XYChart>

    {/** 
      * Tooltip is `html`-based so should be rendered outside the chart `svg`
      * It has access to `tooltipData` and `tooltipCoords` from context.
      */}
    <Tooltip renderInPortal={boolean} />

    {/** 
       * Legend is `html`-based so should be rendered outside the chart `svg`. 
       * It has access to all series via `dataRegistry` from context, 
       * or we could add a legend renderer registry.
       */}
    <Legend  />
  </ChartProvider>
)

The same functionality could be provided in a component API:

import { ChartProvider, XYChart, Legend, Tooltip } from '@vx/xy-chart`

() => (
  <ChartProvider theme={...} {...scaleConfig} >
    <XYChart {...} />
    <Legend {...} />
    <Tooltip {...} />
  </ChartProvider>
)

cc @hshoff @techniq @kristw

[techniq]

First, excellent writeup! You concisely and completely covered very well.

Question, in the Proposed API example, the Fragment wrapping everything should be a ContextProvider (maybe call it ChartProvider or maybe even just Chart)?.

I figure we'll want to start working on a PoC (maybe in codesandbox or something)?

[techniq]

Another question, what about Hierarchical layouts (Treemap, Partition (Sunburst, Icicle), Tree, Pack, Sankey, etc). I assume instead of XYChart we would have HierarchyChart or similar? Also for Radial variants, does it make sense to still call it XYChart. For Semiotic they have XYFrame, OrdinalFrame, and NetworkFrame.

[kristw]

Another question, what about Hierarchical layouts (Treemap, Partition (Sunburst, Icicle), Tree, Pack, Sankey, etc). I assume instead of XYChart we would have HierarchyChart or similar? Also for Radial variants, does it make sense to still call it XYChart. For Semiotic they have XYFrame, OrdinalFrame, and NetworkFrame.

Good questions. I propose this RFC should limit the scope to XYChart since it is already a large enough topic, to prevent scope creeping.

[techniq]

@kristw Agreed, and if the plan is to handle it as a separate component (HierarchyChart/etc) we can do that in the future.

[williaster]

Question, in the Proposed API example, the Fragment wrapping everything should be a ContextProvider (maybe call it ChartProvider or maybe even just Chart)?.

Yes! My mistake, I've updated it to ChartProvider. I think that we'll likely need both ChartProvider and XYChart since things like <Legend /> and <Tooltip /> shouldn't be inside the svg. I guess they could be children of XYChart and be rendered in portals 🤔 but that seems complicated for legend placement.

I figure we'll want to start working on a PoC (maybe in codesandbox or something)?

Also agree, I think a POC sandbox should be easy to create from the vx codesandbox examples integration. I'll start a branch / PR and post back here 🎉

what about Hierarchical layouts ... I assume instead of XYChart we would have HierarchyChart? ... for Radial variants, does it make sense to still call it XYChart? for Semiotic they have XYFrame, OrdinalFrame, and NetworkFrame

Love the idea of a HierarchyChart in the future after we get this one working. Would need to think about Radial variants inside XYChart vs a separate RadialChart, or combining them into a single Chart. I think that Semiotic made the distinction between XY, Ordinal, and Network as fundamental data primitives, which make animating between different Series for a given frame easier (e.g., Bar series + Pie series in an Ordinal frame are fundamentally the same pieces of data with different visual representations). Maybe we should consider XY vs Ordinal ourselves 🤔 cc @emeeks

[kristw]

Features

• Handles mouse events for the XYChart across all data series
• Positions tooltips and provides tooltip data
• Supports programmatic tooltip control
• Mouse events can be defined at chart or series level, can be disabled at series level
• Supports the following *Series which are mostly wrappers around vx: Bar, Line, Point, Area, AreaDifference, StackedArea, StackedBar, GroupedBar, BoxPlot, ViolinPlot, Interval

• Exports CrossHair component for use with Tooltip
• Exports XAxis and YAxis components from vx
• Supports horizontal and vertical reference lines
• Supports x- and y-gridlines
• Supports brush functionality (pre @vx/brush)
• Supports styles with LinearGradient, Patterns, and a chart theme via props
• Wraps individual points in FocusBlur handlers that are a11y accessible / tab-able

• re-written in TypeScript 🔥
• responsive by default 🔥
• supports arbitrary datum shape + x / y data accessors (currently requires { x, y } datum shape) 🔥
• first-class hooks support 🔥

All the good stuffs!

Comment-1

Computes and provides x- and y- scales across all data series

I would like to propose one change from the original @data-ui/xy-chart library. The original library takes scale config object and pass it to factory functions from @vx/scale to create a scale. I wish the XYChart would take D3 scales instead. This way the logic for constructing a scale would be left up to the developer whether to use @vx/scale or not.

Full disclosure: I have been developing the encodable library with a createScaleFromConfig() function, which take the scale config object with similar schema to vega and vega-lite and return a D3 scale. This has more features that the current @vx/scale factories (such as ensuring scales starting at zero, nicing with specified time units, creating piecewise scales that matches number of domain and range items for sequential color scales.) Currently, I ended up having to reverse engineer the output D3 scales back to @vx/scale config objects (which supports a subset of the configuration) to be able to use XYChart.

Comment-2

data provided at the DataSeries level vs the chart container level

Here are a few additional alternatives. I supposed the data mentioned in the RFC is always thought of as an array, but perhaps it does not have to be.

Option 3a. The data have to be passed twice, with the provided coding convention below.

const dataset = {
  cost: [{ x: 1, y: 2, datum: ... }, { x: 3, y: 4, datum: ... }],
  revenue: [{ x: 1, y: 2, datum: ... }, { x: 3, y: 4, datum: ... }],
};

return (
  <XYChart dataset={dataset} >
    <LineSeries key="cost" data={dataset[cost]} />
    <LineSeries key="revenue" data={dataset[revenue]} />
  </XYChart>
);

Option 3b. The data is passed once to the chart, with series referring to its data via key or dataKey (if provided).

<XYChart dataset={{
  cost: [{ x: 1, y: 2, datum: ... }, { x: 3, y: 4, datum: ... }],
  revenue: [{ x: 1, y: 2, datum: ... }, { x: 3, y: 4, datum: ... }],
}} >
  // implicitly use "cost" as dataKey
  <LineSeries key="cost" />
  // implicitly use "revenue" as dataKey
  <LineSeries key="revenue" />
  // Use the same data with the line series above.
  <BarSeries key="revenue-bar" dataKey="revenue" />
</XYChart>

Pros

• Chart have access to all data in order to provide x- and y- scales.
• Can directly link data to the series that will visually represent it
• Does not force all DataSeries to have the same data length.
• One-way, top-down dataflow.
• (3a) More transparency for the developer about the data passed to the chart vs series.

Cons

• The data has to be processed to compute x and y instead of being able to define xAccessor and yAccessor at the Series level. On the bright side, this will push caching/memoization one level above and results are sharable between series that use the same data.
• (3a) Looks a bit duplicative. Perhaps can be refined to be more concise somehow.
• (3b) Series have to access dataset from XYChart parent, but perhaps this is not really a con since it needs access to scales anyway.

Edit: The dataset could go into ChartDataProvider instead if legends and tooltips are needed.

[kristw]

Comment-3

Now the XYChart magically computes and sets domain for the entire chart based on all given data. There might be times when developers wish to override the domain computation, such as to add min/max bounds, zeroing, manually specify domain. Could we add one more optional hook to the XYChart props which sets the default value to the current behavior.

type Props = {
  ...,
  computeDomain: ({ dataset, xScale, yScale }) => void;
}

<XYChart 
  dataset={...} 
  xScale={...} 
  yScale={...} 
  computeDomain={({ dataset, xScale, yScale }) => { 
    // computes and sets xScale.domain(...)
    // computes and sets yScale.domain(...)
  }} 
/>

[williaster]

I would like to propose one change from the original @data-ui/xy-chart library. The original library takes scale config object and pass it to factory functions from @vx/scale to create a scale. I wish the XYChart would take D3 scales instead. This way the logic for constructing a scale would be left up to the developer whether to use @vx/scale or not.

Hmm, to me this adds a lot of eng effort + a learning curve which detracts a bit from the goal of "easy to use out of the box". If you pass in a scale directly, the chart has to either mutate or copy it, which I don't think is as simple as passing in scale config and having the chart create the right scale.

I think the scale config in @data-ui is a little black-box because it wasn't in TypeScript. Maybe we can add some of the config customizations you are thinking about to the config? (would this be a lot of work to port over from encodable?) I think it's reasonable for the chart to use @vx/scale if it's a vx package (which really are just d3 scales).

Now the XYChart magically computes and sets domain for the entire chart based on all given data. There might be times when developers wish to override the domain computation, such as to add min/max bounds, zeroing, manually specify domain. Could we add one more optional hook to the XYChart props which sets the default value to the current behavior.

XYChart currently allows you to pass any d3-scale/@vx/scale config you want, including a custom range or domain for this purpose: xScale={{ type: 'linear', domain: [35, 100] }}. I think exposing the config directly is more straight forward than adding a prop for each config hook (like computeDomain, computeRange).

[kristw]

Hmm, to me this adds a lot of eng effort + a learning curve which detracts a bit from the goal of "easy to use out of the box". If you pass in a scale directly, the chart has to either mutate or copy it, which I don't think is as simple as passing in scale config and having the chart create the right scale.

I am not sure which one is easier to learn. If vx = react + D3, I supposed the developers are already familiar with D3.

import { scaleLinear } from 'd3-scale';
<XYChart xScale={scaleLinear().domain([0, 100]).nice(true)} />

vs. learning another construct which is specific to vx in the current implementation.

<XYChart xScale={{ type: 'linear', domain: [0, 100], nice: true }} />

I am not saying we strongly remove this, but perhaps it can be opt-in.

import { scaleLinear } from 'vx-scale';
<XYChart xScale={scaleLinear({ domain: [0, 100], nice: true })} />

---

XYChart currently allows you to pass any d3-scale/@vx/scale config you want, including a custom range or domain for this purpose: xScale={{ type: 'linear', domain: [35, 100] }}. I think exposing the config directly is more straight forward than adding a prop for each config hook (like computeDomain, computeRange).

This is related to the above proposal if the XYChart accept D3Scale instance (not just a config) then the fixed domain information is not available. e.g. if scale.domain() returns [0, 1], it is not clear if this is the default domain or the fixed domain specified by user.

Example scenarios:

• Example 1: want to ensure that y-axis starts from 0 and max is max value from dataset. Don't see how to configure in config. https://github.com/hshoff/vx/blob/master/packages/vx-scale/src/scales/linear.ts
• Example 2: want the domain to be [0, 95th percentile]
• Example 3: want to exclude some series from domain computation.
• Example 4: want the domain to always be [0, 1] (such as percentage)

Arguably for example 1-3, the domain could be computed beforehand and passed into XYChart as part of the xScale and yScale. Not sure how to deal with the edge case in example 4.

This is probably an advanced feature tho so usually new developers won't touch. Can drop this part from the discussion for v1 scope.

---

(would this be a lot of work to port over from encodable?)

There are possibilities. The part that make it a bit tangle is ability to lookup color scheme by names in encodable. If removed that part, can probably port over.

[techniq]

Regarding custom scales with percentages, we do this in our apps as well, either as 0-100%...

or with 100% as a baseline...

we also have a yScaleZero prop to always show 0 and not "zoom to fit" the data. We do not use XYChart in data-ui but have our own Charts using vx primatives, but would love to move to this composable components as well.

There are also some primatives we have such as OverlayBandBar that is used to 'highlight' on hover that will be much easier and composible when the scales are available via context (and not be required to pass in directly

interface OverlayBarProps {
  x: number;
  xScale: ScaleBand<any>;
  yScale: ScaleLinear<any, any>;
}

function OverlayBandBar(props: OverlayBarProps) {
  const { x, xScale, yScale } = props;

  return (
    <Bar
      x={x - (xScale.padding() * xScale.step()) / 2}
      y={0}
      width={xScale.step()}
      height={yScale.range()[0]}
      fill="rgba(0,0,0,.1)"
    />
  );
}

We also have OverlayTimeBandBar, OverlayTimeBar, and OverlayTimeLine to do similar based on scales.

[williaster]

^awesome examples @techniq 😍 I agree we should support all of these, it should be straight forward with context. Wanted to update that I have a working POC (sandbox link):

I'll open a PR soon for ease of commenting. I'm trying to work out more of the details for events. Some initial notes:

Mapping events <> Datum

data-ui supports different event triggers: container, voronoi, and series. These are useful under different scenarios: voronoi works well for points + lines but not when there are series where single points take up a lot of space like Bar or BoxPlot. Here series works but container is better for e.g., small Bars with small mouse targets. The implementation of container looks for the closest x,y point, but this really still doesn't work well for bars or when supporting e.g., vertical + horizontal bars since these take.

General user feedback (from non-vis engineers) was that the 3 options were confusing and they didn't understand when to use what 😢 So I'm thinking about how we could improve the dev experience by doing this automatically. I think generally we could

• collect all data { x, y }s and use a d3-quadtree for fast lookup at the container level.
• for cases like Bar or HorizontalBar, we could come up with a mechanism for the Series to override the default logic 🤔 still thinking.

Event context

In thinking more about perf, since any child who uses a given context updates when a context value changes, I think it might be preferable to have a separate context for events, which <Tooltip /> or other components could tap into separately from ChartContext. This would have the benefit of not updating every ChartContext consumer from updating every mouse move.

[techniq]

Regarding tooltips and bar charts, one thing I do is render a BarTarget under actual Bar to allow the target to be the full band (instead of having dead zones between the bars, and to go the the top of the chart height.

For example on the our BarChart, we render a <VerticalBarShape> which includes both the visible shape and the hit target (see the band example above and you get the idea of the hit target).

const VerticalBarShape: React.FunctionComponent<VerticalBarShapeProps> = ({
  bar,
  index,
  xScale,
  yScale,
  color,
  barProps,
  barTargetProps,
  showTooltip,
  hideTooltip
}) => {
  const [yMax] = yScale.range();

  return (
    <React.Fragment key={`bar-${bar.x}`}>
      <Bar
        x={bar.x}
        y={bar.y}
        width={bar.width}
        height={bar.height}
        fill={color || bar.color || blue[500]}
        {...(barProps && barProps({ bar, index }))}
      />
      <BarTarget
        bar={bar}
        x={bar.x - (xScale.padding() * xScale.step()) / 2}
        y={0}
        width={xScale.step()}
        height={yMax}
        showTooltip={showTooltip}
        hideTooltip={hideTooltip}
        // fill="rgba(255,0,0,.1)"
        {...(barTargetProps && barTargetProps({ bar, index }))}
      />
    </React.Fragment>
  );
};

we also have a HorizontalBarShape

const HorizontalBarShape: React.FunctionComponent<HorizontalBarShapeProps> = ({
  bar,
  index,
  xScale,
  yScale,
  color,
  barProps,
  barTargetProps,
  showTooltip,
  hideTooltip
}) => {
  const [xMin, xMax] = xScale.range();

  return (
    <React.Fragment key={`bar-${bar.x}`}>
      <Bar
        x={bar.x}
        y={bar.y}
        width={bar.width}
        height={bar.height}
        fill={color || bar.color || blue[500]}
        {...(barProps && barProps({ bar, index }))}
      />
      <BarTarget
        bar={bar}
        x={0}
        y={bar.y - (yScale.padding() * yScale.step()) / 2}
        width={xMax}
        height={yScale.step()}
        showTooltip={showTooltip}
        hideTooltip={hideTooltip}
        {...(barTargetProps && barTargetProps({ bar, index }))}
      />
    </React.Fragment>
  );
};

lastly, sometimes we override the "full band" targets where it makes sense, for example on a TimelineChart (Gnatt chart). It this case we just wante the target to be the size of the bar (plus the circles on the sides which gives it a "pill" shape.

<Fragment key={`${y(d)}-${i}`}>
  <HorizontalBarShape
    bar={bar}
    index={i}
    xScale={xScale}
    yScale={yScale}
    color={color ? color(d, i) : undefined}
    barProps={barProps}
    barTargetProps={({ bar, index }) => ({
      x: bar.x - pointRadius,
      width: bar.width + pointRadius * 2,
      ...(barTargetProps && barTargetProps({ bar, index }))
    })}
    showTooltip={showTooltip}
    hideTooltip={hideTooltip}
  />

  {endpoints && (
    <>
      <circle
        key={'start-' + start.toString()}
        fill={color ? color(d, i) : undefined}
        cx={xScale(start)}
        cy={yScale(y(d)) + pointRadius}
        r={pointRadius}
        style={{ pointerEvents: 'none' }}
        {...(endpointProps &&
          endpointProps({ point: start, index: 0, bar }))}
      />

      <circle
        key={'end-' + end.toString()}
        fill={color ? color(d, i) : undefined}
        cx={xScale(end)}
        cy={yScale(y(d)) + pointRadius}
        r={pointRadius}
        style={{ pointerEvents: 'none' }}
        {...(endpointProps &&
          endpointProps({ point: end, index: 1, bar }))}
      />
    </>
  )}
</Fragment>

I'm not sure how helpful these snippets are but sadly I don't have an example in codesandbox right now.

---

Regarding the separate event context, that seems like a good idea for performance reason (unless we can do a lot of memoization / etc, but taking away as many foot guns as possible is always good.

[williaster]

Wanted to get resolution on a couple of high-level directions while I continue on the POC:

Resolution on data at the Chart container level versus the DataSeries level

@kristw proposed a couple more options for pushing data to the Chart level. I think this is appealing in the sense that you might avoid the need for Series to pass data back up to the Chart container, but I still think that we should opt for data at the DataSeries level.

I think the major limitation at the Chart level, even if you use an object instead of arrays, would be that you to either

• have the same x,y accessors for all data, or
• set them on individual DataSeries

The first point is quite limiting, and the second feels a little disjointed / inconsistent. Additionally, if you are going to add accessors to DataSeries you'd need a mechanism to register them with the Chart container ... and at that point you still need the data registry and should just co-locate the data + accessors.

Resolution on scales vs scale config

@kristw was proposing passing scales directly over configuration and gave this set of examples

Example scenarios:
Example 1: want to ensure that y-axis starts from 0 and max is max value from dataset. Don't see how to configure in config. https://github.com/hshoff/vx/blob/master/packages/vx-scale/src/scales/linear.ts
Example 2: want the domain to be [0, 95th percentile]
Example 3: want to exclude some series from domain computation.
Example 4: want the domain to always be [0, 1] (such as percentage)

Arguably for example 1-3, the domain could be computed beforehand and passed into XYChart as part of the xScale and yScale. Not sure how to deal with the edge case in example 4.

I would still advocate for scale config (with types for easier use/dev experience) for 2 reasons:

1. Config like the following would allow you to implement any of the 4 examples by specifying your own domain. I think this is more straight forward than a user passing in a scale where it's unclear what should be mutated by the Chart vs what has already been set by the user. Perhaps we could also allow you to pass in a scale directly but that seems trickier to type check.

// this is always true
type ScaleOutput = number[];

// in reality we probably need to make this a union of different configs per type
interface ScaleConfig<ScaleInput> {
  type: 'linear' | 'band' | 'time' | ... ;
  domain?: ScaleInput[];
  range?: ScaleOutput[];
  rangeRound?: ScaleOutput[];
  nice?: boolean;
  clamp?: boolean;
}

2. The second reason mostly comes down to developer effort + this question:

I am not sure which one is easier to learn. If vx = react + D3, I supposed the developers are already familiar with D3.

I think most vx users are actually not familiar with d3. Many of the issues that are opened come down to not understanding d3 behavior, so I think having them use config versus having them learn d3 scales would be preferable / simpler.

Object config feels more like react to me and is significantly more readable than the d3 api, which is very foreign to newcomers used to react. Additionally this saves the developer the effort of having to import another thing from the package and use it correctly versus. If the config exposes all of the d3 configuration options I think a d3 developer should be able to easily pick it up.

---

wdyt?

[kristw]

RE: Resolution on data at the Chart container level versus the DataSeries level

@kristw proposed a couple more options for pushing data to the Chart level. I think this is appealing in the sense that you might avoid the need for Series to pass data back up to the Chart container, but I still think that we should opt for data at the DataSeries level.

I feel less strongly about this one and can go either way. My main concern was the challenges for library maintainer to handle the performance optimization, but can see why it can be more developer-friendly. (And since the maintainer is willing to handle the complexity, I will not discourage. 😁 )

Notes:

• The container/registry can do some state management to avoid duplicate datasets from series to optimize computing time.
• Have to do more selector/memoize to avoid updating every series/rendering when the context changes.
• There will be edge case for computing extents when the series have multiple x/y-values anyway. Perhaps the per-series approach is better for this. Implementation-wise, each series have more options to feed into the registry.

RE: Resolution on scales vs scale config

Object config feels more like react to me and is significantly more readable than the d3 api, which is very foreign to newcomers used to react. Additionally this saves the developer the effort of having to import another thing from the package and use it correctly versus...

Agree with making it prioritized for react-oriented developers, but also would like to have support for advanced use cases and/or open the door for d3-oriented developers. We should be able to do the best of both worlds?

I personally ran into the situation that I already have a D3 scale and had to choose between (a) convert my D3 scale to the config, only for it to be reconstructed as a D3 scale again with subset of the behavior I had in my original scale (b) not using XYChart at all. Wish these were not the choices that were given.

@kristw was proposing passing scales directly over configuration

Let me clarify a bit. My original proposal is NOT getting rid of the config, but extends the xScale and yScale fields to also accept D3 scales somehow. I was tossing around the idea which can be taken out of context. Let me reframe it here.

Option A: The current behavior

type XScale = {
  type: 'linear' | 'band' | 'time' | ... ;
  domain?: ScaleInput[];
  range?: ScaleOutput[];
  rangeRound?: ScaleOutput[];
  nice?: boolean;
  clamp?: boolean;
}

<XYChart xScale={{ type: 'linear', ... }} />

Cons: I can't use my own D3 scale and workarounds are not satisfying.

Option B: Also allow D3 scales as input

import {
  ScaleLinear,
  ScaleLogarithmic,
  ScalePower,
  ScaleTime,
  ScalePoint,
  ScaleBand,
} from 'd3-scale';

type SupportedD3Scale =
  | ScaleLinear
  | ScaleLogarithmic
  | ScalePower
  | ScaleTime
  | ScalePoint
  | ScaleBand

type XScale = {
  type: 'linear' | 'band' | 'time' | ... ;
  domain?: ScaleInput[];
  range?: ScaleOutput[];
  rangeRound?: ScaleOutput[];
  nice?: boolean;
  clamp?: boolean;
} | SupportedD3Scale;

// You can pass config
<XYChart xScale={{ type: 'linear', ... }} />
// or You can pass scale.
<XYChart xScale={scaleLinear()} />

Pros: No breaking change.

Cons: Unclear if the input scale should update domain from the data. Needs a way to specify that.

Option C: Only allow D3 scales as input

This is a slight variation of the Option B.

// when using config
import { createScale } from '@vx/scale')
<XYChart xScale={createScale({ type: 'linear' })} />

// when using scale
import { scaleLinear } from 'd3-scale')
<XYChart xScale={scaleLinear()} />

Pros: uniform input for the XYChart component.

Cons:

• Breaking behavior.
• Extra call to import @vx/scale.
• Unclear if the input scale should update domain from the data. Needs a way to specify that.

Option D: Let this be another type

type XScale = {
  type: 'linear' | 'band' | 'time' | ... ;
  domain?: ScaleInput[];
  range?: ScaleOutput[];
  rangeRound?: ScaleOutput[];
  nice?: boolean;
  clamp?: boolean;
} | {
  type: 'custom',
  scale: SupportedD3Scale;
  updateDomain: boolean | (scale, domainFromData) => SupportedD3Scale,
  updateRange: boolean | (scale, dimension) => SupportedD3Scale,
}

<XYChart xScale={{ 
  type: 'custom'
  scale: myOwnScale, 
  updateDomain: false, 
  updateRange: true
}} />

I think this looks safe. The custom type sounds a bit weird, but I am OK with this more than being boxed by the config.

My preference at the moments is D, followed by B with a few more additional props to handle domain/range updates but that will double the number of props because xScale and yScale need their own.

RE: Other smaller points

Checking if an input is a scale config or D3 scale.

Perhaps we could also allow you to pass in a scale directly but that seems trickier to type check.

Can check if .domain field exists and if it is a function.

@vx/scale config can do everything a D3 scale does

If the config exposes all of the d3 configuration options I think a d3 developer should be able to easily pick it up.

This requires @vx/scale to maintain 100% feature parity with d3-scale, which is not the case at the moment. e.g. the unknown function. I am not saying the unknown function is very important, but the point is there will be a time when they are out of sync and the developer will be like what am I supposed to do if I want the new feature.

If you ensure 100% parity and the developer should send PRs/wait for next release, that signals a forever contract for the maintenance of @vx/scale for every new/change features in d3 scales. And if they are exactly the same, then why can't a developer pass a D3 scale if he/she already have it by other means.

Making @vx/scale better

// this is always true
type ScaleOutput = number[];

// in reality we probably need to make this a union of different configs per type
interface ScaleConfig<ScaleInput> {
  type: 'linear' | 'band' | 'time' | ... ;
  domain?: ScaleInput[];
  range?: ScaleOutput[];
  rangeRound?: ScaleOutput[];
  nice?: boolean;
  clamp?: boolean;
}

I have written all of these already in
https://github.com/apache-superset/encodable/blob/master/packages/encodable/src/types/Scale.ts

I can take over @vx/scale and do major updates too if you are OK with it.

[techniq]

Something else to consider about scales is animating. I've been experimenting with this again (mostly with hierarchy charts but also zooming scales for time series, etc) you can see here:

• https://codesandbox.io/s/github/techniq/vx-hierarchy-examples/.

Currently I interpolate/tween the values using d3-interpolate and call scale.domain(...).range(...) in the Spring onFrame function. There might be a better approach for some of the caresian layours, but for Sunburst, I think this currently is the best approach.

I haven't checked in some of my recent changes (working on a zoomable Treemap and adding other hierarchy layouts, still just experimenting though).

I'm hoping to uncover a good pattern / custom animated scale hook for this at some point.

[techniq]

I just had a chance to look at the vx-brush example as I thought it would be similar to animation, although it looks like you build a new scale instance (utilizing useMemo() and deps) instead of mutating the original instance. I guess this would port to a scale config since you would just be sending a new config to vx-chart whenever the brush changes. Not sure this could work for animations though since react-spring makes these changes outside the react render updates (for performance reasons).

[techniq]

I just pulled out an AnimatedScale that better shows this (and cleans up some duplication). I'm not fully settled on the API yet (still experimenting and interpolating with multiple scales (transforms, etc) has some tearing when done individually it appears.

[williaster]

@kristw I think it'd be awesome to improve the typings in @vx/scale, those are a lot of work so would love your help with that and improving the package more generally 😄

@techniq thanks for highlighting more of the complexities with animations and scale updates, I agree we should flesh that out more. your approach is really interesting, that seems like a pretty good API to me overall (we should try to update the brush example to use that approach, we're definitely just brute-forcing it right now 😬 ).

A problem I've not known how to solve with updating scale domain/range is inducing a child component update. In an XYChart setting where the Provider has the scales, my initial thought was that you'd need to create new scale instances in order to properly trigger children to re-render with the new values. It looks like in your hook only the interpolate value would be re-created each cycle? May need to noodle on this more.

---

Separately, it'd be sweet to add patterns/things like useAnimatedScale to @vx/scale over time to make it more distinct and useful in react land than the existing d3 wrappers. @hshoff and I also were contemplating exporting memoized scales, with dependencies, since you often need them in render methods (esp function components) and have to always do the memoization yourself (or face the perf consequences of not doing that).

[techniq]

@williaster With interpolate, it's leveraging react-spring to skip the re-renders. This is also why you have to use animated.* for the DOM (animated.rect, animated.g, etc). This is mentioned breifly on their docs. It's also one of the big benefits over react-move which did these changes with state-only.

I know the react-spring organization also has their own state management call zustand and one of their features is Can inform components transiently (without causing render) and they also do not use react context. I've not used it myself or know much more about it though (and it's more for global state ala-redux if I'm not mistaken). Might be worth looking at their implementation and/or using it ourselves (not sure if we want the depenedency though).

I also just read up a little more as I was curious how zustand works with Concurrent React. It sounds like they are still debating but there was some promise with a new hook added to React (useMutableSource) although it sounds like they are still having issues with it. We might be able to use this hook directly though.

Here's a comparison of different state solutions and if they will work with Concurrent Mode. I'm not using Concurrent Mode yet myself, but thought we'd want to plan for it (and at least not pin ourselves in a corner).

---

Regarding memoizing, I've been waiting to mention this, but it seems like we should memoize all our hierarchy layouts such as Treemap, Partition, etc. I think this is were the slight delay in my examples is coming from after clicking on a child (it's recalculating the layout on each render). As that point we could also expose them as hooks useTreemap()/etc.

[techniq]

There is also react-tracked that sounds promosing. It also looks to have been ported to useMutableSource.

[williaster]

With interpolate, it's leveraging react-spring to skip the re-renders. This is also why you have to use animated.* for the DOM (animated.rect, animated.g, etc). This is mentioned breifly on their docs. It's also one of the big benefits over react-move which did these changes with state-only.

I'm familiar with the use of animated.* in react-spring, my question was more around how, if the scale domain and range change in a high-level component or context provider, without the scale instance changing, how that could trigger the necessary update in say, a react BarSeries component nested below which uses animated under the hood. but maybe that all just works? 🤔 may need to try to get this working in the POC to see.

react-tracked

nice, hadn't seen this one. They note

React context by nature triggers propagation of component re-rendering if a value is changed. To avoid this, this libraries use undocumented feature of calculateChangedBits. It then uses a subscription model to force update when a component needs to re-render.

I wonder if this is actually relevant for us. If we have width/height, or data or scale changes, it does seem like the consumers should update. Our context should be limited to just the chart components consuming it (one per chart), so I would think that the scaling problem wouldn't be a major concern. Animation seems like the biggest concern since you'd be updating values very quickly, but this seems like it should be solved/solvable in combination with react-spring not optimizing the context updates?

useMutableSource

I hadn't seen this either, this definitely seems like a good avenue to go for concurrent mode. I've played around with it once and it was a mess so haven't returned yet. If we do go with context, it seems like this wouldn't be necessary? But definitely good to think about future-proof wise and not cornering ourselves as you say.

---

it seems like we should memoize all our hierarchy layouts such as Treemap, Partition, etc.

LOVE THIS IDEA ❤️

---

I have a bit more time today so will get the PR up for the POC, and try to play with animation.

[williaster]

Have several pieces working with a different animation approach, didn't quite get to the PR today

[techniq]

@williaster

[williaster]

Okay finally! (sorry there were some problems with other vx packages that I fixed locally, that then wouldn't work in the sandbox)

Here's the PR #745 🎉 Have a lot of things working (love the dark mode + animation!), really curious about your feedback on the react-spring approach. I tried this approach and your useAnimatedScale variant but couldn't actually get that to interpolate/tween values correctly. Can share that exploration if you want.

[williaster]

Going to close this issue as implementation is underway. You can follow progress in the project tracker.