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

Performant way to update scales? #608

Closed
cilphex opened this issue Jan 23, 2020 · 6 comments
Closed

Performant way to update scales? #608

cilphex opened this issue Jan 23, 2020 · 6 comments

Comments

@cilphex
Copy link

cilphex commented Jan 23, 2020

I'm trying to make a chart where the scale's domain is adjustable through a slider (input[type="range"]) beneath the chart. (Like this, for example.)

The problem that I am running into is that using setState is not performant, as I would like updates to happen every few ms as the slider is dragged, and not just about once per second.

With setState, I'd do something like this:

onRangeChange(e) {
  const newMaxDays = // calculation
  this.setState({ maxDays: newMaxDays })
}

render() {
  data = someData()

  const xScale = scalePower({
    range: [0, width],
    domain: [0, this.state.maxDays],
    exponent: 0.5
  })

  return (
    <div>
      <svg>
        <LinePath
          data={data}
          x={(d) => xScale(d.index)}
          y={(d) => yScale(d.price)}
        />
      </svg>
      
      <input
        type="range"
        min={0}
        max={100}
        defaultValue={50}
        onChange={(e) => this.onRangeChange(e)}
      />
    </div>
  )

This example ^ is slow and laggy. As the user drags the range slider, the chart scale updates, but it is very choppy.

The more performant alternative is to use the innerRef property on the line path so I can access it directly, and then use native d3 logic to set the scale. But I'm having trouble figuring out how to do this.

constructor(props) {
  this.lineRef = React.createRef();
  this.xScale = null
  this.yScale = null
}

onRangeChange(e) {
  const newMaxDays = // calculation

  // TODO: apply the updated xScale to the view... but how??
  // The following doesn't work...

  this.xScale.domain([0, newMaxDays])

  const line = d3.line()
    .x(d => this.xScale(d.index))
    .y(d => this.yScale(d.price))

  d3.select(this.lineRef.current)
    .attr('d', line)
}

render() {
  data = someData()

  this.xScale = scalePower({
    range: [0, width],
    domain: [0, maxDays],
    exponent: 0.5
  })

  this.yScale = ...

  return (
    <div>
      <svg>
        <LinePath
          innerRef={this.lineRef}
          data={data}
          x={(d) => this.xScale(d.index)}
          y={(d) => this.yScale(d.price)}
        />
      </svg>
      
      <input
        type="range"
        min={0}
        max={100}
        defaultValue={50}
        onChange={(e) => this.onRangeChange(e)}
      />
    </div>
  )

Any tips or is there a different preferred way to do this?

@cilphex
Copy link
Author

cilphex commented Jan 24, 2020

Figured out a way that works. In onRangeChange, needed to change this:

d3.select(this.lineRef.current)
  .attr('d', line)

To this:

d3.select(this.lineRef.current)
  .datum(data)
  .attr('d', line)

Would love any suggestions on how to further simplify/react-ify this code if possible. Will close since I found something that works for me.

@cilphex cilphex closed this as completed Jan 24, 2020
@cilphex
Copy link
Author

cilphex commented Jan 24, 2020

It would be nice to be able to pull the existing d3 line out of the component and re-apply it, rather than having to create a new line.

e.g. Instead of this:

  const line = d3.line()
    .x(d => this.xScale(d.index))
    .y(d => this.yScale(d.price))

  d3.select(this.lineRef.current)
    .attr('d', line)

It would be nice to be able to do something like this:

  d3.select(this.lineRef.current)
    .attr('d', this.outerRef.d3line)

  // ...

  <LinePath
    ref={this.outerRef}
    innerRef={this.lineRef}
  />

Because the attached d3 line isn't really changing, it's just being re-applied while the scale that its x function references has been updated.

@williaster
Copy link
Collaborator

williaster commented Jan 24, 2020

Hi @cilphex 👋 thanks for checking out vx. Perf in react land can indeed be a tricky thing. Your solution does mix mental models of d3 vs react manipulating the dom, which is why we created vx so I hope we can get a "pure vx/react" solution 😄 (and I thiiiink that your approach might result in subtle bugs because once you modify the d attribute of the ref, the dom is actually different from what react thinks it is).

I'm curious if a "pure react" version like the following would work, basically you are using the children render function for full control over the LinePath rendering and to get access to the line/path generator, and only updating the scales + domain when needed (note I haven't tested this, just wrote it up so might be errors I didn't catch). It may still re-initialize the line, but might be worth a shot to try and leverage the children render function override.

function MyLine({ width, height, data }) {
  const [maxDays, setMaxDays] = useState(/** initialMaxDays */);
  
  // recompute scales only if chart dims or data change
  const yScale = useMemo(() => /** compute yScale */, [data, height]);
  
  const xScale = useMemo(() => scalePower({
    range: [0, width],
    domain: [0, maxDays],
    exponent: 0.5
  }), [data, width]);

  useEffect(() => {
    // update the x domain when maxDays changes
    xScale.domain([0, maxDays])
  }, [maxDays]);

  return (
    <>
      <input {...} onChange={() => setMaxDays(...)} />
      <svg {...}>
        <LinePath>
          {({ path }) => (
            // get an empty default path, override x and y
            path.x(d => xScale(d.index));
            path.y(d => yScale(d.price));

            return (
              <path d={path(data) || ''} {...} />
            );
          )}
        </LinePath>
      </svg>
    </>
  );
}

@williaster
Copy link
Collaborator

williaster commented Jan 24, 2020

I would also note that for animations in vx we have had pretty good success with react-spring which allows you to use the react animated library to modify dom element attributes directly (like a path d attribute) rather than going through react's diffing lifecycle and allows for significantly better perf for animations or rapid updates.

typically re-creating objects (like scales) is not the expensive part, it's react's lifecycle.

@cilphex
Copy link
Author

cilphex commented Jan 31, 2020

@williaster Just saw your comments, I'll give them a try soon. My React knowledge might be out of date/inadequate, as I'm not familiar with useEffect yet.

typically re-creating objects (like scales) is not the expensive part, it's react's lifecycle.

^ Yep, that's what I'm discovering! Any way I can update vx components in the "react way" without using setState is what I'm looking for.

@cilphex
Copy link
Author

cilphex commented Jan 31, 2020

@williaster I think LinePath.children is missing from the documentation?

Also curious how you would apply this solution to the Grid component. It also has no children property according to the documentation. (But now I'm not sure if that's because the documentation is lacking or if it really doesn't.)

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