-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
refactor(dashboards): Convert GenericWidgetQueries consumers to use hook #106284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(dashboards): Convert GenericWidgetQueries consumers to use hook #106284
Conversation
Update all consumers of GenericWidgetQueries to use the new useGenericWidgetQueries hook instead of the render props pattern. Changes: - issueWidgetQueries: Use hook directly, maintain MemberListStore logic - spansWidgetQueries: Use hook, preserve confidence information handling - widgetQueries: Extract hook logic into separate component to work with OnDemandControlConsumer - releaseWidgetQueries: Use hook, maintain custom error message logic - traceMetricsWidgetQueries: Use hook with disabled/loading state handling All tests passing.
Remove the GenericWidgetQueries component wrapper and default export since all consumers have been converted to use the useGenericWidgetQueries hook. Also removed the GenericWidgetQueriesProps type and updated releaseWidgetQueries to use UseGenericWidgetQueriesProps in its custom comparator. All tests passing.
…idgetQueries Remove optional api, organization, selection, and queue props from useGenericWidgetQueries hook. The hook now exclusively uses internal React hooks (useApi, useOrganization, usePageFilters, useWidgetQueryQueue). Updated all consumer components (WidgetQueries, IssueWidgetQueries, SpansWidgetQueries, TraceMetricsWidgetQueries, ReleaseWidgetQueries) to not accept or pass these props. Updated all test files to initialize PageFiltersStore for custom selections and pass organization through render options instead of component props.
…zoom Restore minimal override capability for useGenericWidgetQueries hook to support widget viewer modal zoom functionality. Only the selection prop is needed as an override - api, organization, and queue are removed since they use the same hooks/context internally. The widget viewer modal maintains local modalSelection state that changes when users zoom on charts, requiring it to override the global PageFiltersStore selection. All other props (api, organization, queue) were unnecessary as they would access the same context/hooks. Updated all consumer components (IssueWidgetQueries, ReleaseWidgetQueries, WidgetQueries) to only accept optional selection override.
Fix AddToDashboardModal tests by ensuring selection prop is passed through the entire widget rendering chain: WidgetCard -> WidgetCardChartContainer -> WidgetCardDataLoader -> widget query components. This allows the modal to use different selections based on the selected dashboard's saved filters. Previously, WidgetCardDataLoader had selection in its Props type but used Omit<Props, 'selection'> in the function signature, so it was never actually passed through. Also initialize PageFiltersStore in addToDashboardModal.spec.tsx to provide default selection for widget query hooks. Fixes 2 test failures in addToDashboardModal.spec.tsx.
…raceMetricsWidgetQueries Ensure selection prop is consistently passed through the entire widget rendering chain for all widget types. Previously, SpansWidgetQueries and TraceMetricsWidgetQueries were missing the selection prop, so they always used the global PageFiltersStore value instead of the selection passed from parent components. Now all widget query components (IssueWidgetQueries, ReleaseWidgetQueries, WidgetQueries, SpansWidgetQueries, TraceMetricsWidgetQueries) accept an optional selection prop that can override the hook-based selection. This ensures consistent behavior across all widget types for features like: - Widget viewer modal zoom functionality - Dashboard saved filters in AddToDashboardModal All tests continue to pass.
…es and TraceMetricsWidgetQueries" This reverts commit 85078b9.
static/app/views/dashboards/widgetCard/releaseWidgetQueries.tsx
Outdated
Show resolved
Hide resolved
1. Fix customDidUpdateComparator to compare selection changes - Release widgets now properly refetch when page filters change - Modal zoom functionality works correctly for release widgets 2. Restore selection prop to SpansWidgetQueries and TraceMetricsWidgetQueries - These components now support modal zoom functionality - Selection prop is passed through widgetCardDataLoader
…n ReleaseWidgetQueries The customDidUpdateComparator was failing to detect selection changes when propsSelection is undefined, preventing release widgets from refetching data when dashboard page filters are updated. The component resolves selection from either propsSelection (for modal zoom) or hookPageFilters.selection (for normal dashboard), but was passing the unresolved propsSelection to useGenericWidgetQueries. This meant the custom comparator always received undefined and couldn't detect page filter changes. Fix: Pass the resolved selection variable instead of propsSelection.
narsaynorath
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, but I just noticed some stuff about typing selection: any and a selection comparison we're making in the release widget queries component
static/app/views/dashboards/widgetCard/releaseWidgetQueries.tsx
Outdated
Show resolved
Hide resolved
| onDataFetched?: (results: OnDataFetchedProps) => void; | ||
| queue?: WidgetQueryQueue; | ||
| // Optional selection override for widget viewer modal zoom functionality | ||
| selection?: any; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be any? Looks like it could've been the page filter type
static/app/views/dashboards/widgetCard/traceMetricsWidgetQueries.tsx
Outdated
Show resolved
Hide resolved
static/app/views/dashboards/widgetCard/releaseWidgetQueries.tsx
Outdated
Show resolved
Hide resolved
gggritso
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace loose 'any' type with proper PageFilters type for the selection prop across all widget query components. This provides better type safety and makes it clearer what the selection prop should contain. Also fix customDidUpdateComparator in ReleaseWidgetQueries: - Use isSelectionEqual instead of isEqual to properly handle the utc field which has undefined/null/boolean inconsistency issues - Update comment to clarify that custom comparator completely replaces default comparison logic (hook doesn't handle selection separately) - Handle both defined and undefined selection values correctly Files updated: - genericWidgetQueries.tsx - issueWidgetQueries.tsx - releaseWidgetQueries.tsx - spansWidgetQueries.tsx - traceMetricsWidgetQueries.tsx - widgetQueries.tsx

Update all consumers of GenericWidgetQueries to use the new useGenericWidgetQueries hook instead of the render props pattern. No need to pass down org, selection, api, etc, now that we are using a hook pattern.
This resolves some tech debt and moves us closer to using
useQueryinstead of the current approach