fix(k8s): resolve react-hooks/rules-of-hooks in KubeObject by ipsitapp8 · Pull Request #5324 · kubernetes-sigs/headlamp

ipsitapp8 · 2026-05-01T04:24:38Z

Description

This PR resolves the react-hooks/rules-of-hooks ESLint violation in KubeObject.ts where React hooks were being called directly inside static class methods.

To fix this without blowing up the diff or touching the 30+ files that rely on these methods across the codebase, this PR:

Extracts the Hook Logic: Moves the core logic of useApiList, useList, useGet, and useApiGet out of KubeObject and into standalone, exported top-level functions (useKubeApiList, useKubeList, etc.).
Maintains Public APIs: Replaces the original static methods with simple wrapper methods that delegate to the newly exported hooks (e.g., return useKubeList(this, ...args)).
Restores Lint Integrity: Removes all the inline // eslint-disable-next-line react-hooks/rules-of-hooks suppressions.

Why this approach?

Zero Call-Site Updates: Components like PodList can continue doing Pod.useList() identically.
Reviewer-Friendly: Uses simple, standard method delegation rather than complex getter magic or closures. The eslint-plugin-react-hooks rule natively allows static method delegation to hooks as long as the method name starts with use.
Type Safe: TypeScript signatures and ReturnType constraints are strictly preserved.

How to test

Run npm run lint and verify there are no longer any react-hooks/rules-of-hooks errors in KubeObject.ts.
Run npm run tsc to verify TypeScript builds cleanly.
Test any resource list view (e.g., Pods or Namespaces) to confirm the data fetching still functions as expected.

This fix addresses the Storybook test timeout by preventing an infinite re-render loop in the ReleaseNotes component. The appConfig listener is now registered only once on mount, and the AbortController is handled via a Ref to avoid state-induced re-runs of the effect. Ensures Storybook tests can complete within the 5000ms timeout.

k8s-ci-robot · 2026-05-01T04:24:46Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ipsitapp8
Once this PR has been reviewed and has the lgtm label, please assign yolossn for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

illume

Thanks for this PR.

A few of the commits don't quite follow the project guidelines. We use Linux kernel style for git commits — have a look at the contributing guide and previous commits with git log.

Commit guidelines

Use atomic commits focused on a single change.
Use the title format <area>: <description of changes>.
Keep the title under 72 characters (soft requirement).
Explain the intention and why the change is needed.
Make commit titles meaningful and describe what changed.
Do not add code that a later commit rewrites; squash or reorder commits instead.
Do not include Fixes #NN in commit messages.

Good examples:

frontend: HomeButton: Fix so it navigates to home
backend: config: Add enable-dynamic-clusters flag

Copilot

Pull request overview

This PR aims to eliminate react-hooks/rules-of-hooks violations in frontend/src/lib/k8s/KubeObject.ts by moving hook implementations into top-level exported hook functions while keeping the existing KubeObject.*use* static APIs as delegating wrappers.

Changes:

Refactors KubeObject.useApiList, KubeObject.useList, and KubeObject.useGet to delegate to new exported hooks (useKubeApiList, useKubeList, useKubeGet).
Introduces exported hook utilities in KubeObject.ts (including useKubeApiGet).
Refactors desktop Release Notes fetching/abort flow to use an AbortController stored in a ref.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
frontend/src/lib/k8s/KubeObject.ts	Extracts hook bodies into exported top-level hooks and keeps static `use*` methods as wrappers to address hooks linting.
frontend/src/components/common/ReleaseNotes/ReleaseNotes.tsx	Refactors update-check/release fetching logic to use a ref-held `AbortController` and simplifies effect dependencies.

Copilot · 2026-05-01T13:07:04Z

  React.useEffect(() => {
-    if (desktopApi) {
-      desktopApi.receive(
-        'appConfig',
-        (config: { appVersion: string; checkForUpdates: boolean }) => {
-          const { appVersion: currentBuildAppVersion, checkForUpdates = true } = config;
-          if (!checkForUpdates) {
-            console.debug("Skipping update check because config's checkForUpdates is false");
-            return;
+    if (!desktopApi) {
+      return;
+    }
+
+    desktopApi.receive('appConfig', (config: { appVersion: string; checkForUpdates: boolean }) => {
+      const { appVersion: currentBuildAppVersion, checkForUpdates = true } = config;


desktopApi.receive('appConfig', ...) registers a listener but the effect doesn’t remove it on unmount. Since desktopApi appears to support removeListener elsewhere, consider storing the handler function in a const and returning a cleanup that removes the listener (and aborts any in-flight fetch via controllerRef) to avoid leaks / setState-on-unmounted-component cases.

Copilot · 2026-05-01T13:07:04Z

 export default function ReleaseNotes() {
  const { desktopApi } = window;
  const [releaseNotes, setReleaseNotes] = React.useState<string>('');
  const [releaseDownloadURL, setReleaseDownloadURL] = React.useState<string | null>(null);
  const [fetchingRelease, setFetchingRelease] = React.useState<boolean>(false);
  const [releaseFetchFailed, setReleaseFetchFailed] = React.useState<boolean>(false);
-  const [skipFetch, setSkipFetch] = React.useState(false);
-
-  // network controller this makes sure if the github request is still lying around we
-  // abort it on fetch release skip press button click
-  const controller = new AbortController();
-  const signal = controller.signal;
+  const controllerRef = React.useRef<AbortController | null>(null);


The PR description is focused on fixing react-hooks/rules-of-hooks violations in KubeObject.ts, but this file has a non-trivial refactor of the GitHub release fetching/abort logic as well. Please either update the PR description to cover this behavior change or split it into a separate PR to keep scope focused.

Copilot · 2026-05-01T13:07:04Z

+export function useKubeApiList<K extends KubeObject>(
+  kubeObjectClass: (new (...args: any) => K) & typeof KubeObject<any>,
+  onList: (...arg: any[]) => any,
+  onError?: (err: ApiError, cluster?: string) => void,
+  opts?: ApiListOptions


useKubeApiList is currently preceded by a JSDoc block that describes makeKubeObject (deprecated/params/returns). Because JSDoc attaches to the next symbol, this ends up documenting useKubeApiList incorrectly and leaves makeKubeObject undocumented. Please move that JSDoc down to makeKubeObject (or replace it with a correct doc for useKubeApiList).

Copilot · 2026-05-01T13:07:05Z

+  // We do the type conversion here because we want to be able to use hooks that may not have
+  // the exact signature as get callbacks.
+  const getCallback = onGet as (item: K) => void;
+  useConnectApi(kubeObjectClass.apiGet(getCallback, name, namespace, onError, opts));
+}


There is still a // eslint-disable-next-line react-hooks/rules-of-hooks in KubeObject.useApiGet (around line 378) calling useConnectApi(...), so the original lint violation isn’t fully resolved. Since useKubeApiGet is now introduced, please update static useApiGet to delegate to useKubeApiGet (and remove the suppression) the same way useApiList/useList/useGet were updated.

Copilot · 2026-05-01T13:07:05Z

+       * Also sets the following state whilst running:
+       * - fetchingRelease
+       * - releaseFetchFailed
+       * - skipFetch


This comment still lists skipFetch as managed state, but the skipFetch state was removed in this refactor. Please update the comment to reflect the actual state being set (or remove the bullet).

Suggested change

* - skipFetch

illume

Thanks for the contribution.

There are some open Copilot review comments — could you take a look at them?

The frontend lint job in CI is failing. Run cd frontend && npm run lint to see the errors.

How to fix lint errors

Run cd frontend && npm run lint to see all ESLint errors. Many can be fixed automatically with cd frontend && npm run lint -- --fix. Remaining errors need manual attention.

ipsitapp8 added 2 commits April 27, 2026 13:46

fix(k8s): Refactor KubeObject hooks out of static methods

ed7472c

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label May 1, 2026

k8s-ci-robot requested review from vyncent-t and yolossn May 1, 2026 04:24

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 1, 2026

illume requested a review from Copilot May 1, 2026 13:01

illume requested changes May 1, 2026

View reviewed changes

Copilot started reviewing on behalf of illume May 1, 2026 13:02 View session

Copilot AI reviewed May 1, 2026

View reviewed changes

illume requested changes May 3, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(k8s): resolve react-hooks/rules-of-hooks in KubeObject#5324

fix(k8s): resolve react-hooks/rules-of-hooks in KubeObject#5324
ipsitapp8 wants to merge 2 commits intokubernetes-sigs:mainfrom
ipsitapp8:fix-issue-5242

ipsitapp8 commented May 1, 2026

Uh oh!

k8s-ci-robot commented May 1, 2026

Uh oh!

illume left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI May 1, 2026

Uh oh!

Copilot AI May 1, 2026

Uh oh!

Copilot AI May 1, 2026

Uh oh!

Copilot AI May 1, 2026

Uh oh!

Copilot AI May 1, 2026

Uh oh!

illume left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

ipsitapp8 commented May 1, 2026

Description

Why this approach?

How to test

Uh oh!

k8s-ci-robot commented May 1, 2026

Uh oh!

illume left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 1, 2026

Choose a reason for hiding this comment

Uh oh!

illume left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants