Skip to content

fix: Fix crash when navigating away from chart explorer search page#1278

Merged
pulpdrew merged 1 commit intomainfrom
drew/fix-series-editor-crash
Oct 17, 2025
Merged

fix: Fix crash when navigating away from chart explorer search page#1278
pulpdrew merged 1 commit intomainfrom
drew/fix-series-editor-crash

Conversation

@pulpdrew
Copy link
Copy Markdown
Contributor

@pulpdrew pulpdrew commented Oct 16, 2025

Fixes HDX-2619

This PR fixes a full page crash whenever navigating away from the Chart Explorer search tab to another page. The issue was caused by the use of useFieldArray in the DBEditTimeChart component on a field (select) which can be either an array or a string. That hook is known to crash when used on fields which are not arrays of objects.

Crash example
Screen.Recording.2025-10-17.at.10.35.33.AM.mov
Fixed behavior
Screen.Recording.2025-10-17.at.10.35.08.AM.mov

The fix was to maintain a separate field (series) which is always an array. When select is an array, series matches select. Prior to saving or submitting the chart configuration, the series value is removed and stored in the select field (unless the select field is a string, which is the case on the search tab of chart explorer).

Validated:

✅ All tile types work in dashboards, without any change to the storage format (series is not persisted as a separate field)
✅ All chart explorer tabs work, without any change to the URL parameter format (series is not persisted as a separate URL parameter field)
✅ We aren't using useFieldArray anywhere else in the app
✅ AI Chart Assistant can still set the chart

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Oct 16, 2025

🦋 Changeset detected

Latest commit: 13d0833

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@hyperdx/app Patch
@hyperdx/api Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link
Copy Markdown

vercel bot commented Oct 16, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
hyperdx-v2-oss-app Ready Ready Preview Comment Oct 17, 2025 3:00pm

@claude
Copy link
Copy Markdown

claude bot commented Oct 16, 2025

Code Review

Reviewed the fix for HDX-2619 (chart explorer crash). The approach is sound - maintaining a separate series field that is always an array to work around useFieldArray's limitations.

Issues Found:

  • Zod schema mismatch (packages/app/src/components/DBEditTimeChartForm.tsx:436) - The form uses SavedChartConfigWithSeries type (which includes the series field) but validates with zSavedChartConfig which only validates the alert field via .passthrough(). The series field will not be validated, and the select field type mismatch will not be caught. Update the Zod schema to properly validate SavedChartConfigWithSeries or create a separate schema.

  • Type safety issue (packages/app/src/components/DBEditTimeChartForm.tsx:519) - config.select.length assumes array but select can still be a string at this point if displayType === DisplayType.Search. Should use Array.isArray(config.select) check instead.

  • Mutation of form data (packages/app/src/components/DBEditTimeChartForm.tsx:552-555) - handleSave mutates the form value v directly (v.select = ...). Should create a new object instead to avoid side effects.

Validation status: The PR correctly maintains backward compatibility and handles the state transitions, but the validation layer needs attention.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Oct 16, 2025

E2E Test Results

All tests passed • 25 passed • 3 skipped • 213s

Status Count
✅ Passed 25
❌ Failed 0
⚠️ Flaky 0
⏭️ Skipped 3

View full report →

@pulpdrew pulpdrew force-pushed the drew/fix-series-editor-crash branch from 266dfc0 to 13d0833 Compare October 17, 2025 14:57
@pulpdrew pulpdrew marked this pull request as ready for review October 17, 2025 14:57
@pulpdrew pulpdrew requested review from a team and brandon-pereira and removed request for a team October 17, 2025 15:05
Copy link
Copy Markdown
Member

@brandon-pereira brandon-pereira left a comment

Choose a reason for hiding this comment

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

Tested locally, worked well and issue not observed. Nice work!

@pulpdrew pulpdrew merged commit 6262ced into main Oct 17, 2025
10 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