Skip to content

Conversation

@aaryan610
Copy link
Member

@aaryan610 aaryan610 commented Nov 14, 2025

Description

This PR refactors instances where hooks are initialized conditionally.

Type of Change

  • Code refactoring

Summary by CodeRabbit

  • Refactor

    • Improved filter initialization logic for consistent handling of undefined states across multiple components.
    • Optimized conditional hook invocation patterns to ensure filters are properly defined in all code paths.
  • Chores

    • Added comprehensive codebase analysis documenting hook usage patterns and conditional filter initialization approaches.

@makeplane
Copy link

makeplane bot commented Nov 14, 2025

Linked to Plane Work Item(s)

This comment was auto-generated by Plane

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 14, 2025

Walkthrough

This PR introduces a useSWR hooks usage analysis report and systematically refactors work-item filter initialization across empty state components. The changes move undefined-checks from component call sites into the useWorkItemFilterInstance hook itself, enabling unconditional hook invocations with the hook handling undefined entity IDs.

Changes

Cohort / File(s) Change Summary
New Report
CONDIDENTIAL_USESWR_REPORT.md
Introduces Markdown report analyzing useSWR hook usage patterns; catalogs 4 files violating Rules of Hooks (hooks before early returns) and ~92 files following correct conditional-key patterns; includes remediation recommendations.
Hook Signature Update
apps/web/core/hooks/store/work-item-filters/use-work-item-filter-instance.ts
Modified useWorkItemFilterInstance to accept entityId: string | undefined and return undefined when entityId is undefined; previously always invoked getFilter().
Empty State Components
apps/web/core/components/issues/issue-layouts/empty-states/archived-issues.tsx,
apps/web/core/components/issues/issue-layouts/empty-states/cycle.tsx,
apps/web/core/components/issues/issue-layouts/empty-states/module.tsx,
apps/web/core/components/issues/issue-layouts/empty-states/project-issues.tsx
Replaced conditional hook invocations with unconditional calls; filters are now always initialized via hooks regardless of ID availability, delegating undefined-check logic to the hook.
Calendar Component Refactor
apps/web/core/components/issues/issue-layouts/calendar/base-calendar-root.tsx
Introduced local fallbackStoreType variable and refactored storeType selection logic; functionally equivalent with improved readability.

Sequence Diagram

sequenceDiagram
    participant Comp as Empty State Component
    participant Hook as useWorkItemFilterInstance
    participant Filter as Filter Store

    rect rgba(100, 150, 200, 0.3)
    Note over Comp,Filter: Before: Conditional Hook Invocation
    alt ID exists
        Comp->>Hook: call hook(id)
        Hook->>Filter: getFilter(id)
        Filter-->>Hook: filter instance
        Hook-->>Comp: filter instance
    else ID undefined
        Note over Comp: skip hook call
        Comp->>Comp: filterInstance = undefined
    end
    end

    rect rgba(100, 150, 200, 0.3)
    Note over Comp,Filter: After: Unconditional Hook Invocation
    Comp->>Hook: call hook(id or undefined)
    alt ID exists
        Hook->>Filter: getFilter(id)
        Filter-->>Hook: filter instance
    else ID undefined
        Hook->>Hook: early return undefined
    end
    Hook-->>Comp: filter instance or undefined
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Hook signature change: Verify that the new undefined-handling logic in useWorkItemFilterInstance correctly propagates through dependent components and doesn't introduce unintended side effects.
  • Systematic component updates: Ensure all four empty state components handle undefined filter returns safely (note: code shows optional chaining is used, but confirm no downstream assumptions of non-null filters).
  • New report artifact: Review the useSWR violations and recommended patterns to confirm accuracy and assess implications of flagged files.

Poem

🐰 Hooks now hop where paths align,
No early leaps—just conditional signs,
Filters initialize without a fuss,
Rules of Hooks kept righteous for us!
thump thump

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description is incomplete; it lacks required sections including detailed explanation, test scenarios, and references, though it does specify the type of change correctly. Expand the description to include: specific examples of changed components, test scenarios validating the refactoring, and links to related issues (e.g., WEB-5429).
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main refactoring objective—converting conditionally initialized hooks to unconditionally initialized ones—which aligns with the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/conditional-hooks

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4e357c4 and a54f0c4.

📒 Files selected for processing (7)
  • CONDITIONAL_USESWR_REPORT.md (1 hunks)
  • apps/web/core/components/issues/issue-layouts/calendar/base-calendar-root.tsx (1 hunks)
  • apps/web/core/components/issues/issue-layouts/empty-states/archived-issues.tsx (1 hunks)
  • apps/web/core/components/issues/issue-layouts/empty-states/cycle.tsx (1 hunks)
  • apps/web/core/components/issues/issue-layouts/empty-states/module.tsx (1 hunks)
  • apps/web/core/components/issues/issue-layouts/empty-states/project-issues.tsx (1 hunks)
  • apps/web/core/hooks/store/work-item-filters/use-work-item-filter-instance.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-06-16T07:23:39.497Z
Learnt from: vamsikrishnamathala
Repo: makeplane/plane PR: 7214
File: web/core/store/issue/helpers/base-issues.store.ts:117-117
Timestamp: 2025-06-16T07:23:39.497Z
Learning: In the updateIssueDates method of BaseIssuesStore (web/core/store/issue/helpers/base-issues.store.ts), the projectId parameter is intentionally made optional to support override implementations in subclasses. The base implementation requires projectId and includes an early return check, but making it optional allows derived classes to override the method with different parameter requirements.

Applied to files:

  • apps/web/core/components/issues/issue-layouts/empty-states/project-issues.tsx
  • apps/web/core/components/issues/issue-layouts/calendar/base-calendar-root.tsx
  • apps/web/core/components/issues/issue-layouts/empty-states/archived-issues.tsx
📚 Learning: 2025-07-08T13:41:01.659Z
Learnt from: prateekshourya29
Repo: makeplane/plane PR: 7363
File: apps/web/core/components/issues/select/dropdown.tsx:9-11
Timestamp: 2025-07-08T13:41:01.659Z
Learning: The `getProjectLabelIds` function in the label store handles undefined projectId internally, so it's safe to pass undefined values to it without explicit checks in the calling component.

Applied to files:

  • apps/web/core/components/issues/issue-layouts/empty-states/project-issues.tsx
  • apps/web/core/components/issues/issue-layouts/empty-states/archived-issues.tsx
📚 Learning: 2025-06-18T09:46:08.566Z
Learnt from: prateekshourya29
Repo: makeplane/plane PR: 7188
File: web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/cycles/(list)/header.tsx:40-45
Timestamp: 2025-06-18T09:46:08.566Z
Learning: When reviewing breadcrumb components that accept projectId or similar props, check if empty strings are being passed during loading states, which can result in invalid URLs. The preferred approach is to handle these loading states internally within the component rather than requiring each consumer to manage the loading logic.

Applied to files:

  • apps/web/core/components/issues/issue-layouts/empty-states/project-issues.tsx
📚 Learning: 2025-10-21T17:22:05.204Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.

Applied to files:

  • apps/web/core/components/issues/issue-layouts/calendar/base-calendar-root.tsx
🧬 Code graph analysis (6)
apps/web/core/components/issues/issue-layouts/empty-states/project-issues.tsx (1)
apps/web/core/hooks/store/work-item-filters/use-work-item-filter-instance.ts (1)
  • useWorkItemFilterInstance (7-13)
apps/web/core/hooks/store/work-item-filters/use-work-item-filter-instance.ts (2)
packages/shared-state/src/store/work-item-filters/shared.ts (1)
  • IWorkItemFilterInstance (8-8)
apps/web/core/hooks/store/work-item-filters/use-work-item-filters.ts (1)
  • useWorkItemFilters (7-11)
apps/web/core/components/issues/issue-layouts/empty-states/module.tsx (2)
apps/web/core/hooks/store/work-item-filters/use-work-item-filter-instance.ts (1)
  • useWorkItemFilterInstance (7-13)
apps/web/core/store/issue/helpers/base-issues.store.ts (1)
  • moduleId (264-266)
apps/web/core/components/issues/issue-layouts/calendar/base-calendar-root.tsx (1)
apps/web/core/hooks/use-issue-layout-store.ts (1)
  • useIssueStoreType (8-37)
apps/web/core/components/issues/issue-layouts/empty-states/archived-issues.tsx (1)
apps/web/core/hooks/store/work-item-filters/use-work-item-filter-instance.ts (1)
  • useWorkItemFilterInstance (7-13)
apps/web/core/components/issues/issue-layouts/empty-states/cycle.tsx (2)
apps/web/core/hooks/store/work-item-filters/use-work-item-filter-instance.ts (1)
  • useWorkItemFilterInstance (7-13)
apps/web/core/store/issue/helpers/base-issues.store.ts (1)
  • cycleId (269-271)
🪛 LanguageTool
CONDITIONAL_USESWR_REPORT.md

[uncategorized] ~202-~202: The official name of this software platform is spelled with a capital “H”.
Context: ... - apps/web/core/components/integration/github/root.tsx - apps/web/core/components/int...

(GITHUB)


[uncategorized] ~203-~203: The official name of this software platform is spelled with a capital “H”.
Context: ... - apps/web/core/components/integration/github/select-repository.tsx - apps/web/core/c...

(GITHUB)


[uncategorized] ~207-~207: The official name of this software platform is spelled with a capital “H”.
Context: ... - apps/web/core/components/integration/github/single-user-select.tsx - apps/web/core/...

(GITHUB)


[uncategorized] ~208-~208: The official name of this software platform is spelled with a capital “H”.
Context: ... - apps/web/core/components/integration/github/repo-details.tsx - apps/web/core/compon...

(GITHUB)


[grammar] ~236-~236: Ensure spelling is correct
Context: ...nal keys properly The vast majority of useSWR usage in the codebase follows the corre...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)


[grammar] ~291-~291: Ensure spelling is correct
Context: ...>; }; ``` --- ## Key Takeaway Most useSWR usage is correct! The issue is specif...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Agent
  • GitHub Check: CodeQL analysis (javascript-typescript)
  • GitHub Check: Build and lint web apps
  • GitHub Check: Analyze (javascript)
🔇 Additional comments (7)
apps/web/core/components/issues/issue-layouts/calendar/base-calendar-root.tsx (2)

55-56: LGTM: Cleaner extraction of hook invocation.

Extracting the useIssueStoreType() call to a separate variable improves readability and ensures unconditional hook invocation, which aligns with React's Rules of Hooks.


1-299: Excellent documentation of useSWR patterns.

This report provides comprehensive analysis distinguishing correct conditional key usage from Rules of Hooks violations. The examples and remediation patterns are clear and actionable.

Note: The 4 violations documented (in apps/space and workspace/project wrappers) are not addressed in this PR, which focuses on work-item filter hooks. Consider tracking those violations separately if not already planned.

apps/web/core/components/issues/issue-layouts/empty-states/cycle.tsx (1)

39-39: LGTM: Correct application of unconditional hook pattern.

The hook is now invoked unconditionally with cycleId (which may be undefined), and the undefined check is handled within useWorkItemFilterInstance. Downstream usage correctly employs optional chaining (cycleWorkItemFilter?.hasActiveFilters, cycleWorkItemFilter?.clearFilters).

apps/web/core/components/issues/issue-layouts/empty-states/project-issues.tsx (1)

25-25: LGTM: Consistent unconditional hook pattern.

The hook is invoked unconditionally with potentially undefined projectId, with downstream code using optional chaining appropriately (projectWorkItemFilter?.hasActiveFilters, projectWorkItemFilter?.clearFilters).

apps/web/core/components/issues/issue-layouts/empty-states/module.tsx (1)

37-37: LGTM: Consistent unconditional hook pattern.

The hook is invoked unconditionally with potentially undefined moduleId, with downstream code using optional chaining appropriately (moduleWorkItemFilter?.hasActiveFilters, moduleWorkItemFilter?.clearFilters).

apps/web/core/components/issues/issue-layouts/empty-states/archived-issues.tsx (1)

24-24: LGTM: Consistent unconditional hook pattern.

The hook is invoked unconditionally with potentially undefined projectId, with downstream code using optional chaining appropriately (archivedWorkItemFilter?.hasActiveFilters, archivedWorkItemFilter?.clearFilters).

apps/web/core/hooks/store/work-item-filters/use-work-item-filter-instance.ts (1)

7-13: LGTM: Core hook correctly updated and all call sites already follow the new pattern.

All call sites already pass potentially undefined entityId parameters (derived from router params with conditional checks), and all properly handle the undefined return type using optional chaining:

  • moduleWorkItemFilter?.hasActiveFilters
  • projectWorkItemFilter?.clearFilters
  • Nullish checks: !archivedWorkItemFilter

Type safety is maintained throughout the codebase. No additional changes needed at call sites.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot finished reviewing on behalf of aaryan610 November 14, 2025 08:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the handling of conditional hooks to improve adherence to React's Rules of Hooks. The main change is moving conditional logic from call sites into the useWorkItemFilterInstance hook itself, allowing it to be called unconditionally while still handling undefined entity IDs gracefully.

  • Modified useWorkItemFilterInstance to accept string | undefined for entityId and return undefined when entityId is falsy
  • Updated four call sites to remove conditional hook invocations (project-issues, module, cycle, archived-issues)
  • Extracted conditional hook logic in calendar component to avoid calling hooks conditionally

Reviewed Changes

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

Show a summary per file
File Description
apps/web/core/hooks/store/work-item-filters/use-work-item-filter-instance.ts Updated hook signature to accept string | undefined and return undefined when entityId is falsy
apps/web/core/components/issues/issue-layouts/empty-states/project-issues.tsx Removed conditional ternary, now calls hook unconditionally
apps/web/core/components/issues/issue-layouts/empty-states/module.tsx Removed conditional ternary, now calls hook unconditionally
apps/web/core/components/issues/issue-layouts/empty-states/cycle.tsx Removed conditional ternary, now calls hook unconditionally
apps/web/core/components/issues/issue-layouts/empty-states/archived-issues.tsx Removed conditional ternary spanning multiple lines, now calls hook unconditionally
apps/web/core/components/issues/issue-layouts/calendar/base-calendar-root.tsx Extracted useIssueStoreType call to avoid conditionally invoking hooks
CONDITIONAL_USESWR_REPORT.md Added comprehensive documentation analyzing conditional useSWR patterns in the codebase

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sriramveeraghanta sriramveeraghanta merged commit f3031a4 into preview Nov 20, 2025
6 checks passed
@sriramveeraghanta sriramveeraghanta deleted the refactor/conditional-hooks branch November 20, 2025 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants