Skip to content

fix: os-situation metric page crashes when the metric query is not ready - #607

Open
muzimu217 wants to merge 1 commit into
oss-compass:mainfrom
muzimu217:fix/os-situation-metric-not-ready-crash
Open

muzimu217 wants to merge 1 commit into
oss-compass:mainfrom
muzimu217:fix/os-situation-metric-not-ready-crash

Conversation

@muzimu217

Copy link
Copy Markdown

What happens

Opening the OS situation metric page can render a blank screen. When the page crashes, the console shows:

TypeError: Cannot read properties of undefined (reading 'map')

This is not limited to one bad URL. The page reaches the crashing state in three ordinary situations:

  • First client render. The page is wrapped in NoSsr, so it renders on the client only. router.query is still an empty object on that first pass, which means metric is undefined even for a perfectly valid URL such as /os-situation/metrics/push.
  • An unknown metric segment. A stale bookmark or a hand-typed /os-situation/metrics/anything resolves metric to a string that is not a key of the categories map.
  • Any metric other than import_export. DataView/index.tsx special-cases only import_export; every other value falls through to <ChartsLine metric={metric} /> with whatever the router holds.

Why it crashes

useCategoriesData() returns an object with a fixed set of keys:

{ push, import_export, repositories, topics, contributor, language, license }

ChartsLine.tsx looks the route parameter up in that object and then iterates the result without checking it:

const chartsList = categoriesData[metric];   // undefined when metric misses
...
{chartsList.map((ChartItem) => {             // undefined.map -> TypeError

ChartsLine has no error boundary above it, so the throw takes down the whole page.

Why this is an oversight rather than a design choice

The sidebar on the very same page reads the exact same map and already guards it:

// components/SideBar/index.tsx:21
const menuItems = categoriesData[metric]?.map((item: any) => ({ ... }));

SideBar survives the empty router.query and unknown segments; ChartsLine does not. The two components disagree about whether categoriesData[metric] can be undefined, and it clearly can.

The change

-        {chartsList.map((ChartItem) => {
+        {chartsList?.map((ChartItem) => {

One line, following the SideBar precedent. No new helpers, no control-flow rewrite, no behaviour change for a valid metric — a known metric still renders every chart group, an unknown one renders the empty container, which is what the sidebar already shows in that state.

No UI copy was touched, so no i18n/en or i18n/zh updates are needed.

Verification

Before (fix stashed), a component test rendering the real Charts component:

✕ does not crash while the route metric is still undefined
  Error name:    "TypeError"
  Error message: "Cannot read properties of undefined (reading 'map')"
✕ does not crash for an unknown metric segment
  Error name:    "TypeError"
  Error message: "Cannot read properties of undefined (reading 'map')"
✓ still renders every chart group for a known metric
Tests: 2 failed, 1 passed, 3 total

After:

✓ does not crash while the route metric is still undefined
✓ does not crash for an unknown metric segment
✓ still renders every chart group for a known metric
Tests: 3 passed, 3 total

The third case is the important one: it confirms the optional chain does not swallow the normal path.

A dependency-free Node reproduction of the two lines gives the same red/green split, with the push case producing byte-identical output before and after:

metric=undefined          before -> THROW TypeError: Cannot read properties of undefined (reading 'map')
                          after  -> ok: []
metric=not-a-real-metric  before -> THROW TypeError: Cannot read properties of undefined (reading 'map')
                          after  -> ok: []
metric=push               before -> ok: ["global_push"]
                          after  -> ok: ["global_push"]

CI gates

  • yarn test:ci — 32 suites / 138 tests pass (run with TZ=UTC; three date-formatting suites fail on a non-UTC machine both with and without this change, they are pre-existing and timezone-dependent)
  • yarn build — ✓ Compiled successfully
  • tsc --noEmit — no new errors (76 pre-existing, all *.svg module declarations; zero in the changed file)

Scope

Single file, single line: apps/web/src/modules/os-situation/DataView/ChartsLine.tsx.

…undefined when the metric query is not ready
@vercel

vercel Bot commented Oct 20, 2026

Copy link
Copy Markdown

@muzimu217 is attempting to deploy a commit to the codersett's projects Team on Vercel.

A member of the Team first needs to authorize it.

This branch has not been deployed

No deployments
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.

1 participant