Migrate to uplot#3295
Conversation
Getting these in puts us in a spot where we can pretty easily see the differences between uplot and recharts.
This is just a touch more portable.
Verbose, but fairly straightforward. The only meaningful changes you should see are the lack of a top grid line (since uplot does tick calculation, this would often be quite tight with the line below) and uplot's tick selection logic replacing ours (which is for the greater good).
Any canvas drawing element is going to need themes this way. I considered doing something more involved, like computing the style within the subscription, or actually making a the subscription an effect hook, but the thing is, you kind of want to only use getComputedStyle consciously.
If you passed an anonymous function as the yAxisTickFormatter, that would cause rerender cycles that screwed with hovering, because the graph was being constantly re-created. You could say the caller is just responsible for not doing that, but I was feeling generous.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Those are some good ticks. Does this also fix the missing x-axis ticks when there’s a big gap in the data? |
| @import '@oxide/design-system/styles/light.css'; | ||
| @import '@oxide/design-system/styles/preflight.css' layer(base); | ||
| @import 'simplebar-react/dist/simplebar.min.css' layer(components); | ||
| @import 'uplot/dist/uPlot.min.css' layer(components); |
There was a problem hiding this comment.
This seems fine but worth checking it's not overriding something we don't want it to. Like:
.u-legend th {
font-weight: 600;
}
We're not using title or legend here but we might elsewhere.
| const v = (name: string) => style.getPropertyValue(name) | ||
| return { | ||
| fontFamily: v('--font-mono'), | ||
| stroke: v('--content-accent-tertiary'), |
There was a problem hiding this comment.
Was already like this, but we should probably switch to the equivalent semantic stroke token --stroke-accent-secondary
|
Currently not working in the vercel preview, maybe related error:
|
|
Unfortunately wide gamut / // uPlot's canvas defaults to sRGB, which clips our oklch theme colors, so charts look washed out.
// uPlot exposes no way to pass context attributes, so we patch the canvas default globally.
function defaultCanvasToDisplayP3() {
try {
const attrs = document
.createElement('canvas')
.getContext('2d', { colorSpace: 'display-p3' })
?.getContextAttributes?.()
if (attrs?.colorSpace !== 'display-p3') return // browser doesn't support it
} catch {
return
}
const original = HTMLCanvasElement.prototype.getContext
HTMLCanvasElement.prototype.getContext = function (
this: HTMLCanvasElement,
contextId: string,
options?: object
) {
return original.call(this, contextId, { ...options, colorSpace: 'display-p3' })
// cast needed because a plain function can't satisfy getContext's overload set
} as typeof original
}
defaultCanvasToDisplayP3() |
|
I don't think the CSP warning is related, it's there on main too. I think it's from this, which would only affect the preview because I don't think we use zod in production: colinhacks/zod#4461. Still probably worth fixing. |
3 participants
This PR replaces recharts with uPlot. The core of the decision here is really just "stop using an svg-based chart", but uPlot is a fast option even among canvas-based renderers, and (with a bit of patience) still quite flexible when it comes to styling.
Setting the code itself aside, this should not be that noticeable a change. Charts will render faster, but the smaller datasets we're using right now weren't really a problem for recharts; it would be much more noticeable once we started work on #3270. The three most noticeable changes today:
As an example of difference 2 in particular, here's the visual comparison test for instance network metrics: