fix(AuthVisible): move hooks above early return to satisfy Rules of H…

[kashish00208]

Fixes #5187

Changes

Moves useQuery and useEffect above the early return in AuthVisible to ensure hooks are always executed in the same order on every render, fixing the react-hooks/rules-of-hooks ESLint error while preserving existing authorization logic via conditional enabled handling in useQuery.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: kashish00208
Once this PR has been reviewed and has the lgtm label, please assign ashu8912 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.

Commits that need attention

• fix(AuthVisible): move hooks above early return to satisfy Rules of Hooks — Missing area: description prefix — e.g. frontend: HomeButton: Fix so it navigates to home or backend: config: Add enable-dynamic-clusters flag.

Commit guidelines

• Use atomic commits focused on a single change.
• Use the title format <area>: <Description of changes> — description must start with a capital letter.
• 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 fix a react-hooks/rules-of-hooks violation in AuthVisible by ensuring useQuery/useEffect are called before any early returns, while keeping the existing authorization gating via React Query’s enabled flag.

Changes:

• Moved the invalid-authVerb early return below useQuery and useEffect.
• Removed prior per-line ESLint disables for hook rules.

Comments suppressed due to low confidence (4)

frontend/src/components/common/Resource/AuthVisible.tsx:93

• useEffect references visible, but visible is declared after the effect and is never initialized on the invalid-authVerb early-return path. If authVerb is invalid, the component still mounts (renders null) and the effect callback will run later and throw a ReferenceError when it tries to read visible. Initialize visible (and any isValidVerb flag) before the effect and before any return, or compute allowed directly from data inside the effect without referencing a potentially uninitialized variable.

  useEffect(() => {
    if (data) {
      onAuthResult?.({
        allowed: visible,
        reason: data.status?.reason ?? '',
      });
    }

frontend/src/components/common/Resource/AuthVisible.tsx:70

• useQuery is now executed even when authVerb is invalid. Because enabled only checks !!item, an invalid verb will still trigger the authorization request and onAuthResult callbacks (even though the component returns null). To preserve the previous behavior and avoid unnecessary/invalid API calls, include the verb validation in enabled (e.g., enabled: !!item && isValidVerb).

  const { data } = useQuery<any>({
    enabled: !!item,
    queryKey: [
      'authVisible',
      itemName,
      itemClass.apiName,
      itemClass.apiVersion,
      authVerb,
      subresource,

frontend/src/components/common/Resource/AuthVisible.tsx:69

• itemClass is typed as KubeObjectClass | null, but queryKey unconditionally dereferences itemClass.apiName / itemClass.apiVersion. If item is null/undefined at runtime (allowed by the prop type and possible from callsites like namespaces[0]), this will throw during render even though the query is disabled. Guard these fields (optional chaining / fallbacks) and/or ensure enabled also requires a non-null itemClass before building a key that dereferences it.

This issue also appears in the following locations of the same file:

• line 62
• line 87
• line 87

  const itemClass: KubeObjectClass | null = (item as KubeObject)?._class?.() ?? item;
  const itemName = (item as KubeObject)?.getName?.();

  const { data } = useQuery<any>({
    enabled: !!item,
    queryKey: [
      'authVisible',
      itemName,
      itemClass.apiName,
      itemClass.apiVersion,
      authVerb,

frontend/src/components/common/Resource/AuthVisible.tsx:95

• The useEffect dependency array is now [data], but the effect body also uses onAuthResult and (indirectly) authVerb/visible. In CI, react-hooks/exhaustive-deps is enabled and will warn here, and at runtime this can call a stale onAuthResult if the prop changes. Add the missing dependencies (or restructure to avoid capturing them) instead of relying on an incomplete dependency list.

  useEffect(() => {
    if (data) {
      onAuthResult?.({
        allowed: visible,
        reason: data.status?.reason ?? '',
      });
    }
  }, [data]);

[skoeva]

hi, could you address the linter failures? thanks

[kashish00208]

hi, could you address the linter failures? thanks

Thanks , I’ve addressed the linter failures and pushed the updates.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

frontend/src/components/common/Resource/AuthVisible.tsx:69

• The invalid authVerb guard was moved below useQuery, but useQuery is still enabled: !!item. That means an invalid verb will still trigger an authorization request even though the warning says the check is being skipped, and onAuthResult may fire for an invalid verb. Consider gating the query with something like enabled: !!item && VALID_AUTH_VERBS.includes(authVerb) (and similarly avoid side-effects when the verb is invalid).

  const { data } = useQuery<any>({
    enabled: !!item,
    queryKey: [
      'authVisible',
      itemName,
      itemClass.apiName,
      itemClass.apiVersion,
      authVerb,

frontend/src/components/common/Resource/AuthVisible.tsx:69

• item is typed as KubeObject | KubeObjectClass | null, but itemClass can be null when item is null. The queryKey still unconditionally reads itemClass.apiName/apiVersion, which will throw even when enabled is false (because queryKey is built during render). Use optional chaining/defaults in the key and include !!itemClass in enabled, or otherwise ensure safe key construction when item is null.

  const itemClass: KubeObjectClass | null = (item as KubeObject)?._class?.() ?? item;
  const itemName = (item as KubeObject)?.getName?.();

  const { data } = useQuery<any>({
    enabled: !!item,
    queryKey: [
      'authVisible',
      itemName,
      itemClass.apiName,
      itemClass.apiVersion,
      authVerb,

[illume]

Thanks for these changes.

Can you please have a look at the git commits to see if they meet the contribution guidelines? We use a Linux kernel style of git commits. See the contributing guide for general context, and please see previous git commits with git log for examples.

Commits that need attention

• frontend : Fix AuthVisible useEffect — Missing area: description prefix — e.g. frontend: HomeButton: Fix so it navigates to home or backend: config: Add enable-dynamic-clusters flag.

Commit guidelines

• Use atomic commits focused on a single change.
• Use the title format <area>: <Description of changes> — description must start with a capital letter.
• 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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

frontend/src/components/common/Resource/AuthVisible.tsx:72

• Moving the invalid authVerb check below the hooks changes behavior: useQuery can still run with an invalid verb (since enabled is only !!item), and useEffect may still call onAuthResult based on query data even though the component ultimately returns null. It can also now throw before reaching the warning/return when item is null because queryKey dereferences itemClass.apiName/apiVersion. Consider computing isValidAuthVerb up-front and using it to (a) gate the query via enabled, (b) avoid dereferencing itemClass in the queryKey when item is null, and (c) guard the onAuthResult side-effect when the verb is invalid so the prior “skip authorization check” behavior is preserved.

  const { data } = useQuery<any>({
    enabled: !!item,
    queryKey: [
      'authVisible',
      itemName,
      itemClass.apiName,
      itemClass.apiVersion,
      authVerb,
      subresource,
      namespace,
    ],

[kashish00208]

Thanks for these changes.

Can you please have a look at the git commits to see if they meet the contribution guidelines? We use a Linux kernel style of git commits. See the contributing guide for general context, and please see previous git commits with git log for examples.

Commits that need attention

• frontend : Fix AuthVisible useEffect — Missing area: description prefix — e.g. frontend: HomeButton: Fix so it navigates to home or backend: config: Add enable-dynamic-clusters flag.

Commit guidelines

Yes, the Git commits follow the contribution guidelines.

[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.

Commits that need attention

• frontend : Fix AuthVisible useEffect — Missing area: description prefix — e.g. frontend: HomeButton: Fix so it navigates to home or backend: config: Add enable-dynamic-clusters flag.
• frontend : Re-add a justified ESLint disable — Missing area: description prefix — e.g. frontend: HomeButton: Fix so it navigates to home or backend: config: Add enable-dynamic-clusters flag.

Commit guidelines

• Use atomic commits focused on a single change.
• Use the title format <area>: <Description of changes> — description must start with a capital letter.
• 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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

[kashish00208]

@skoeva @illume I've made the requested changes. It should be ready for review now.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

frontend/src/components/common/Resource/AuthVisible.tsx:70

• item is typed as KubeObject | KubeObjectClass | null, but queryKey unconditionally dereferences itemClass.apiName / itemClass.apiVersion. React Query evaluates queryKey during render even when enabled is false, so passing item={null} will still throw at render time. Consider guarding itemClass access (e.g., use optional chaining / fallback values in the key) so the component can safely render when item is null.

    queryKey: [
      'authVisible',
      itemName,
      itemClass.apiName,
      itemClass.apiVersion,

[illume]

Thanks for the contribution.

Could you take a look at the commit messages in this PR? We follow a Linux kernel style for git commits — see the contributing guide and git log for examples.

Commits that need attention

• frontend : Authvisible remove duplicate work — Missing area: description prefix — e.g. frontend: HomeButton: Fix so it navigates to home or backend: config: Add enable-dynamic-clusters flag.

Commit guidelines

• Use atomic commits focused on a single change.
• Use the title format <area>: <Description of changes> — description must start with a capital letter.
• 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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

frontend/src/components/common/Resource/AuthVisible.tsx:70

• item is typed as nullable, but the queryKey unconditionally dereferences itemClass.apiName / itemClass.apiVersion. When item is null, itemClass becomes null and this will throw during render even though the query is disabled. Make the key construction null-safe (e.g., optional chaining / default placeholders) or otherwise guard against item === null without violating the Rules of Hooks.

    queryKey: [
      'authVisible',
      itemName,
      itemClass.apiName,
      itemClass.apiVersion,

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

frontend/src/components/common/Resource/AuthVisible.tsx:71

• item is typed as nullable, but itemClass will be null when item is null, and the queryKey unconditionally reads itemClass.apiName/apiVersion. React Query evaluates queryKey even when enabled is false, so this will throw at render time. Make the queryKey null-safe (e.g., itemClass?.apiName/itemClass?.apiVersion) and consider including !!itemClass in the enabled condition (and/or returning a stable placeholder key) so AuthVisible can safely render with item === null.

  const itemClass: KubeObjectClass | null = (item as KubeObject)?._class?.() ?? item;
  const itemName = (item as KubeObject)?.getName?.();

  const { data } = useQuery<any>({
    enabled: !!item && isValidVerb,
    queryKey: [
      'authVisible',
      itemName,
      itemClass.apiName,
      itemClass.apiVersion,
      authVerb,

[illume]

Thanks for working on this.

Would you mind addressing the open Copilot review comments? Please mark each comment as resolved after addressing it.

[kashish00208]

@illume take a look at this PR every copilot comment is resolved , let me know if any changes are required !

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

[illume]

Thanks for this PR.

The open review comments from Copilot still need attention — can you have a look? Once addressed, please mark them as resolved.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

[illume]

Thanks for these changes.

Can you please address the open review comments? Once you've resolved each one, please mark it as resolved.