[release-4.21] OCPBUGS-86427: Fix Shipwright detail pages crashing with React error #310#16487
[release-4.21] OCPBUGS-86427: Fix Shipwright detail pages crashing with React error #310#16487sg00dwin wants to merge 1 commit into
Conversation
…penshift#310 useShipwrightBreadcrumbsFor is a React hook (6 sub-hooks) that was incorrectly passed as a plain callback via the breadcrumbsFor prop to DetailsPage. ConnectedPageHeading calls breadcrumbsFor conditionally based on data loading state, violating React's Rules of Hooks. On OCP 4.20+, ConnectedPageHeading gained its own hooks, causing React to detect the hook count mismatch between renders and throw error openshift#310: "Rendered more hooks than during the previous render." This is a manual backport of openshift#16474 to release-4.21. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@sg00dwin: This pull request references Jira Issue OCPBUGS-86427, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
|
@sg00dwin: This pull request references Jira Issue OCPBUGS-86427, which is invalid:
Comment DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
|
@sg00dwin: This pull request references Jira Issue OCPBUGS-86427, which is invalid:
Comment DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
|
@sg00dwin: This pull request references Jira Issue OCPBUGS-86427, which is valid. The bug has been moved to the POST state. 7 validation(s) were run on this bug
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@sg00dwin: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/label backport-risk-assessed |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: logonoff, sg00dwin The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Analysis / Root cause:
useShipwrightBreadcrumbsForis a React hook containing 6 sub-hooks (useActivePerspective,useParams,useLocation,useTranslation,useConsoleSelector,useMemo) that was incorrectly passed as a plain callback via thebreadcrumbsForprop toDetailsPage.Inside
ConnectedPageHeading,breadcrumbsForis called conditionally based on data loading state:This violates React's Rules of Hooks. On the first render (data empty), 0 hooks are invoked via
breadcrumbsFor; on re-render (data loaded), 6 hooks are invoked.This bug was latent since commit
e98f1e58b1(Aug 2024, ODC-7632). On OCP 4.19,ConnectedPageHeadinghad no hooks of its own, so React couldn't detect the violation. On OCP 4.20+, commitf803a25cb7added 3 hooks toConnectedPageHeading, causing React to detect the hook count mismatch (3 → 9) and throw error #310: "Rendered more hooks than during the previous render."All 4 Shipwright detail pages are affected: Build, BuildRun, BuildStrategy, and ClusterBuildStrategy.
Linked Jira: https://issues.redhat.com/browse/OCPBUGS-86427
Parent Jira: https://issues.redhat.com/browse/OCPBUGS-84214
Support Cases: 04362500, 04425432, 04430794
Solution description:
Refactored all 4 affected detail pages to call
useShipwrightBreadcrumbsForunconditionally at the component level using their existinguse*Model()hooks, and pass the result as a closure:This follows the established pattern used by the knative plugin (
ServiceDetailsPage.tsx).Also changed
useShipwrightBreadcrumbsForto accept aK8sModeldirectly instead of aK8sResourceKind, and removed the now-deadresourceToModelfunction (30 lines).This is a manual backport of #16474 to release-4.21.
Test setup:
ClusterBuildStrategydeployed (e.g.,buildah— typically installed by the operator automatically)Test cases:
/k8s/cluster/shipwright.io~v1beta1~ClusterBuildStrategy/buildah— page should render without crash, breadcrumbs should show correctlyBrowser conformance:
Backport plan
4.22 (#16485 - merged), 4.21 (this PR), 4.20
Additional info:
shipwright-plugin