unsplash/react-trend

Errors when data array contains single value

wopian opened this issue · 5 comments

wopian commented

Using this to display the mean ratings trend of seasonal shows, so the data is reset with new shows on the first day of each season - leaving only a single value in the array.

Steps to Reproduce

return <Trend data={[1]}/>

Observed Behaviour

  • Errors to console with nothing rendered in DOM (breaks rest of the component from being rendered)

Trend.js?81b8:67 Uncaught TypeError: Cannot read property 'getTotalLength' of undefined
    at Trend.componentDidMount (eval at <anonymous> (bundle.js:3852), <anonymous>:76:34)
    at eval (eval at <anonymous> (bundle.js:3342), <anonymous>:265:25)
    at measureLifeCyclePerf (eval at <anonymous> (bundle.js:3342), <anonymous>:75:12)
    at eval (eval at <anonymous> (bundle.js:3342), <anonymous>:264:11)
    at CallbackQueue.notifyAll (eval at <anonymous> (bundle.js:1819), <anonymous>:76:22)
    at ReactReconcileTransaction.close (eval at <anonymous> (bundle.js:3545), <anonymous>:80:26)
    at ReactReconcileTransaction.closeAll (eval at <anonymous> (bundle.js:1197), <anonymous>:206:25)
    at ReactReconcileTransaction.perform (eval at <anonymous> (bundle.js:1197), <anonymous>:153:16)
    at batchedMountComponentIntoNode (eval at <anonymous> (bundle.js:1875), <anonymous>:126:15)
    at ReactDefaultBatchingStrategyTransaction.perform (eval at <anonymous> (bundle.js:1197), <anonymous>:140:20)

Expected Behaviour

  • A flat line or no graph

markpinero commented

Is this an intended use case for react-trend? I know it's typically used for line graphs. This is the source code of Trend.js on line 118-121 indicating why it won't render:

    // We need at least 2 points to draw a graph.
    if (!data || data.length < 2) {
      return null;
    }
wopian commented

Is this an intended use case for react-trend?

Don't see why it wouldn't be, since I just need a spark line to show up/down trends. Would also be an issue depending on how logging is done for uptime of new servers/activity of new users.

It's more that it has a fairly destructive exit if length < 2

Work around is to just duplicate single values:

return <Trend data={props.data.length > 1 ? props.data : props.data.concat(props.data)}/>
joshwcomeau commented

Ah, right. Yeah, this was an oversight; from a purely mathematical perspective, you can't have a line with only 1 point, so I assumed any attempt to draw a line with a single data point would be a mistake. Never considered data that resets at the start of a period.

That said, I almost feel like a flat line is a little misleading... It gives the impression that the data is flat, when in actuality the trend might be increasing or decreasing from the previous period.

At any rate, I certainly don't think it should crash; my proposed solution would be to just render nothing. We could even have a prop that takes an element to render when not enough data is available, so you could render a fallback:

<Trend
  data={...}
  fallback={<h1>🚫</h1>}
/>

@samijaber thoughts?

samijaber commented

Yeah, a flat line would indeed be inaccurate. The correct behaviour as far as I see it is to not render a graph at all if there's less than 2 points, without crashing/erroring. A fallback could be nice, although part of me feels that it's more the end-user's concern.

  • 👍1
joshwcomeau commented

Gonna close this, as I don't foresee it being resolved any time soon.

As Sami suggests, the user-land solution is pretty simple:

data.length > 0
  ? <Trend data={data} />
  : <SomeFallbackComponent />
  • 👎1
  • 😕1