fix(web): guard unguarded data derefs causing work-item and layout crashes - #9546
Merged
Conversation
Updated the mapping of label IDs to ensure that only defined label options are included in the defaultLabelOptions array across multiple components. This change enhances the robustness of the label handling in the IssueProperties and SpreadsheetLabelColumn components, as well as in the PeekOverviewProperties component.
Updated multiple components to include checks for array types before accessing results. This change enhances stability by preventing potential runtime errors when results are undefined or not an array. Affected components include DescriptionVersionsRoot, PrevExports, SingleIntegrationCard, ProfileActivity, and IssueSubIssuesStore.
Updated the IssueLayoutHOC component to include LayoutErrorBoundary, enhancing error handling by wrapping the children. This change aims to provide a more robust user experience by catching layout-related errors effectively.
codingwolf-at
marked this pull request as draft
August 4, 2026 14:42
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR adds ChangesLayout resilience and defensive data handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant User
participant IssueLayoutHOC
participant LayoutErrorBoundary
participant LayoutView
IssueLayoutHOC->>LayoutErrorBoundary: render children with layout key
LayoutErrorBoundary->>LayoutView: render layout view
LayoutView-->>LayoutErrorBoundary: throw render error
LayoutErrorBoundary-->>User: show localized fallback and retry button
User->>LayoutErrorBoundary: click retry
LayoutErrorBoundary->>LayoutView: remount children
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
React Doctor found 3 new issues in 3 files · 3 warnings · score 91 / 100 (Great) · 1 fixed · vs 3 warnings
Reviewed by React Doctor for commit |
Refactored the handleRefresh function in the PrevExports component to use useCallback, improving performance by memoizing the function. Additionally, updated the useEffect dependency array to include handleRefresh, ensuring the effect runs correctly when dependencies change. This change enhances the efficiency of the component's refresh logic.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
codingwolf-at
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/core/components/integration/single-integration-card.tsx (1)
109-111: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove
anyfrom the integration lookup.The changed callback uses
(i: any). This bypasses strict type checking forintegration_detail. Type the SWR result asIWorkspaceIntegration[], or annotate the callback withIWorkspaceIntegration, and letfindinfer the element type.Proposed fix
- ? workspaceIntegrations.find((i: any) => i.integration_detail.id === integration.id) + ? workspaceIntegrations.find((i: IWorkspaceIntegration) => i.integration_detail.id === integration.id)As per coding guidelines, TypeScript strict mode is enabled and all TSX files must be typed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/core/components/integration/single-integration-card.tsx` around lines 109 - 111, Remove the explicit any annotation from the callback in the isInstalled integration lookup. Type workspaceIntegrations as IWorkspaceIntegration[] or annotate the callback parameter as IWorkspaceIntegration so find infers and validates integration_detail.id under strict TypeScript typing.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/core/store/issue/issue-details/sub_issues.store.ts`:
- Around line 169-170: Update the sub-issue response handling around
subIssuesStateDistribution and subIssues to explicitly validate that response
exists and that response.sub_issues is the expected array shape before either
map call. Handle grouped-object responses through the existing runtime
validation path rather than relying on nullish fallback or a simple
Array.isArray default, and preserve empty results for invalid or missing data.
---
Nitpick comments:
In `@apps/web/core/components/integration/single-integration-card.tsx`:
- Around line 109-111: Remove the explicit any annotation from the callback in
the isInstalled integration lookup. Type workspaceIntegrations as
IWorkspaceIntegration[] or annotate the callback parameter as
IWorkspaceIntegration so find infers and validates integration_detail.id under
strict TypeScript typing.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d8b5265e-e75b-4892-86d8-1fd0cd273e77
📒 Files selected for processing (10)
apps/web/core/components/common/layout-error-boundary.tsxapps/web/core/components/core/description-versions/root.tsxapps/web/core/components/exporter/prev-exports.tsxapps/web/core/components/integration/single-integration-card.tsxapps/web/core/components/issues/issue-layouts/issue-layout-HOC.tsxapps/web/core/components/issues/issue-layouts/properties/all-properties.tsxapps/web/core/components/issues/issue-layouts/spreadsheet/columns/label-column.tsxapps/web/core/components/issues/peek-overview/properties.tsxapps/web/core/components/profile/overview/activity.tsxapps/web/core/store/issue/issue-details/sub_issues.store.ts
…d error messaging Refactored the LayoutErrorBoundary component to include a dedicated LayoutErrorFallback for better error presentation. Added a retry mechanism that allows users to attempt to reload the content after an error occurs. This change improves user experience by providing clearer messaging and a more interactive way to recover from errors.
…ents Refactored the defaultLabelOptions logic in multiple components to use flatMap for better handling of undefined labels. Additionally, updated array checks in the PrevExports component to ensure results are properly validated before access. These changes enhance the robustness and stability of the components, preventing potential runtime errors.
… handling Updated the ProfileActivity component to enhance the loading state management and streamline the rendering of user activity results. The refactor includes a more efficient check for userProfileActivity, ensuring that loading indicators and empty states are displayed correctly. This change improves the user experience by providing clearer feedback during data fetching and handling scenarios with no activity results.
…ub-issues store Updated the SingleIntegrationCard component to use a specific type for workspace integrations, enhancing type safety. Additionally, refactored the subIssues assignment in the IssueSubIssuesStore to ensure it correctly checks for an array before assignment, improving stability and preventing potential runtime errors.
codingwolf-at
marked this pull request as ready for review
August 4, 2026 15:01
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/core/components/exporter/prev-exports.tsx`:
- Around line 49-52: Update handleRefresh to await mutate inside a
try/catch/finally flow, logging the typed error in catch and always calling
setRefreshing(false) from finally. Preserve the existing EXPORT_SERVICES_LIST
request and refresh state initialization.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d51e886e-0b6a-4bfa-b557-9f511d389238
📒 Files selected for processing (9)
apps/web/core/components/common/activity/user.tsxapps/web/core/components/common/layout-error-boundary.tsxapps/web/core/components/exporter/prev-exports.tsxapps/web/core/components/integration/single-integration-card.tsxapps/web/core/components/issues/issue-layouts/properties/all-properties.tsxapps/web/core/components/issues/issue-layouts/spreadsheet/columns/label-column.tsxapps/web/core/components/issues/workspace-draft/draft-issue-properties.tsxapps/web/core/components/profile/overview/activity.tsxapps/web/core/store/issue/issue-details/sub_issues.store.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/web/core/store/issue/issue-details/sub_issues.store.ts
- apps/web/core/components/integration/single-integration-card.tsx
- apps/web/core/components/issues/issue-layouts/spreadsheet/columns/label-column.tsx
- apps/web/core/components/profile/overview/activity.tsx
- apps/web/core/components/issues/issue-layouts/properties/all-properties.tsx
Refactored the handleRefresh function in the PrevExports component to include error handling during the refresh process. The function now uses async/await for better readability and ensures that any errors during the mutation are logged, improving the robustness of the component's refresh logic.
Multi-line flatMap guard needed reformatting to satisfy oxfmt.
Adjusted the formatting of the defaultLabelOptions logic in the DraftIssueProperties component to maintain consistency with the project's coding standards. This change enhances readability without altering functionality.
prateekshourya29
approved these changes
Aug 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Customers reported full-page crashes when opening work items and switching layouts (Slack thread, Aug 4). Traced the crashes to a recurring pattern: components dereferencing API/store data (
.find,.map,.filter, property access) with only a null-guard (?.), not a shape guard — so a stale relation, a malformed/paginated response, or a member with an incomplete record throws deep inside a render and takes the whole page down via the top-level route error boundary.This PR:
LayoutErrorBoundaryaround the issue layout switcher (List/Kanban/Spreadsheet/Calendar/Gantt) so a crash in one layout's render degrades to an inline "Try again" panel instead of crashing the entire app — this contains the still-unreproduced "switching to columns crashes the page" report without requiring an exact repro.Type of Change
Screenshots and Media (if applicable)
NA - no UI change
Test Scenarios
display_name— peek and full detail view should render instead of crashing (peek-overview/properties.tsx).sub_issues/state_distributionin the response.Array.isArrayguards).References
d.find is not a function) was investigated as a possible root cause; the specific code path it pointed to no longer exists in this codebase, but it confirmed the defect class (unguarded.find()on API-sourced data) this PR addresses.Summary by CodeRabbit
New Features
Bug Fixes