Skip to content

console: bump visx to 4 - #39611

Merged
leedqin merged 2 commits into
MaterializeInc:mainfrom
leedqin:console-visx-4
Oct 8, 2026
Merged

leedqin merged 2 commits into
MaterializeInc:mainfrom
leedqin:console-visx-4

Conversation

@leedqin

@leedqin leedqin commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Motivation

Part of moving the Console to React 19. visx 3.12 caps React at 18 in its peer range. Main also mixes @visx/grid and @visx/shape 4.0 with the other visx packages on 3.12, which installs two copies of the shared visx internals.

Supersedes #39496.

Description

  • All @visx/* packages move to 4.0.0, deduped to one copy each.

  • visx 4 adds an exports map, so deep imports such as @visx/tooltip/lib/hooks/useTooltipInPortal and @visx/responsive/lib/components/ParentSize no longer resolve. They now import from the package roots. This includes the vi.mock in ThresholdLineGraph.test.tsx.

  • visx 4's ParentSize puts its children in an absolutely positioned box inside a height: 100% container, so it collapses to 0px tall when its parent has no fixed height. Two charts relied on the old behavior:

    • Cluster overview utilization graphs: they set their own height, so they now measure only their width with useParentSize.
    • Source and sink error graph: its ParentSize gets the graph's fixed height.

    The other five ParentSize call sites are already in fixed-height boxes. The redesigned cluster page's resource usage chart (console: add resource usage charts to the redesigned cluster page #39508) has the same width-only ParentSize, so whichever PR lands second needs this change there.

Verification

Existing tests. jsdom doesn't do layout, so the tests can't catch the collapse. In headless Chromium, the utilization graph renders 0px tall on visx 4 without this change and at full height with it.

Worth eyeballing on the preview:

  • the cluster overview CPU, memory and disk graphs;
  • a source's and a sink's error graph;
  • the freshness graph;
  • the billing spend chart.

🤖 Generated with Claude Code

visx 3.12 caps React at 18, so this is needed for React 19. It also ends
the mix of visx 3 and 4 packages on main.

visx 4 adds an exports map, so deep imports move to the package roots. Its
ParentSize now collapses to 0px tall without a fixed-height parent, so the
cluster utilization graphs measure only their width, and the connector
error graph gives ParentSize its fixed height.
@leedqin
leedqin marked this pull request as ready for review October 8, 2026 19:31
@leedqin
leedqin requested a review from a team as a code owner October 8, 2026 19:31

@antiguru antiguru left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found.

I checked the following against the PR head:

  • visx 4.0.0 declares react: ^18.0.0 || ^19.0.0 as its peer range, so it works with the current React 18.3 and unblocks 19.
  • yarn.lock resolves every @visx/* package, including the internal bounds, point, text, and vendor, to a single 4.0.0 entry, so the mixed 3.12/4.0 install is gone.
  • No @visx/*/lib/... imports remain under console/src.
  • The collapse explanation matches the 4.0.0 source. ParentSize renders an outer div with height: 100% and an absolutely positioned measurement child, so the outer div is 0px tall when its parent has auto height.
  • The other five ParentSize call sites all sit in fixed-height boxes: FreshnessGraph, ThresholdLineGraph, AccountSpendChart, the spend sparkline, and ConnectorStatisticsGraph.
  • In ConnectorErrorsGraph, style is merged into the outer div, so height: HEIGHT_PX lands on the right element.
  • In UtilizationGraph, useParentSize on a full-width Box with minWidth={0} measures the same width as before. ClusterUtilizationGraphInner has no external callers, so narrowing its props type is safe.

Suggestions:

  • #39508 is still open and has the same width-only ParentSize. A comment on #39508 would keep the follow-up from getting lost, since a miss renders a 0px chart that no test catches.
  • jsdom does no layout, so the preview checklist in the description is the only coverage for the error, freshness, and billing charts. It's worth doing before merge.

Nit: the comment at UtilizationGraph.tsx:381 names ParentSize, which this component no longer uses. Something like "visx's ParentSize would collapse here..." reads clearer, or drop it since the PR description carries the reasoning.

Posted by Claude Code on behalf of @antiguru.

@antiguru antiguru left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks. My bot had some nits.

@leedqin
leedqin enabled auto-merge (squash) October 8, 2026 20:33
@leedqin
leedqin merged commit 5c63db6 into MaterializeInc:main Oct 8, 2026
81 checks passed
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

Successfully merging this pull request may close these issues.

2 participants