web-app icon indicating copy to clipboard operation
web-app copied to clipboard

Web 428 memory leaks in multiple components due to unmanaged subscriptions

Open AnvayKharb opened this issue 1 month ago • 1 comments

Resolved memory leaks across multiple components by adding proper RxJS subscription cleanup. Updated each affected component to use the takeUntil(destroy$) pattern and implemented ngOnDestroy() to ensure all active subscriptions are terminated when the component is destroyed. This improves performance, prevents unnecessary background processing, and ensures consistent subscription handling throughout the application.

#Issue Number

WEB-428

Screenshots, if any

N/A

Summary by CodeRabbit

  • Refactor
    • Improved resource management across many UI areas (holidays, product steppers, notifications, account steps).
    • Strengthened subscription cleanup to prevent memory leaks, reducing potential UI slowdowns or unexpected behavior over time.
    • Result: more stable performance and predictable component teardown during navigation and interactions.

✏️ Tip: You can customize this high-level summary in your review settings.

AnvayKharb avatar Nov 21 '25 19:11 AnvayKharb

[!NOTE]

.coderabbit.yaml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)
Validation error: Unrecognized key(s) in object: 'pre_merge_checks'
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Walkthrough

Added OnDestroy lifecycle handling across multiple Angular components: each component now defines a private destroy$ Subject, pipes subscriptions through takeUntil(this.destroy$), and emits/completes destroy$ in ngOnDestroy to ensure unsubscription.

Changes

Cohort / File(s) Summary
Holiday management components
src/app/organization/holidays/create-holiday/create-holiday.component.ts, src/app/organization/holidays/edit-holiday/edit-holiday.component.ts
Implements OnDestroy, adds private destroy$ = new Subject<void>(), pipes route/form/valueChanges and submit/update subscriptions through takeUntil(this.destroy$), and adds ngOnDestroy() to emit/complete destroy$.
Product charge step components
src/app/products/fixed-deposit-products/.../fixed-deposit-product-charges-step.component.ts, src/app/products/loan-products/.../loan-product-charges-step.component.ts, src/app/products/recurring-deposit-products/.../recurring-deposit-product-charges-step.component.ts, src/app/products/share-products/.../share-product-charges-step.component.ts
Components now implement OnDestroy, introduce destroy$ Subject, pipe currencyCode.valueChanges (and other valueChanges like multiDisburseLoan) through takeUntil(this.destroy$), and add ngOnDestroy() to emit/complete destroy$.
Shared notifications & shares account
src/app/shared/notifications-tray/notifications-tray.component.ts, src/app/shares/shares-account-stepper/shares-account-charges-step/shares-account-charges-step.component.ts
Adds destroy$ Subject; wraps forkJoin, API calls, and valueChanges subscriptions with takeUntil(this.destroy$); implements OnDestroy and ngOnDestroy() to emit/complete destroy$.

Sequence Diagram(s)

sequenceDiagram
    participant C as Angular Component
    participant O as Observable (forms, valueChanges, API)
    participant D as destroy$ Subject

    C->>O: subscribe(...).pipe(takeUntil(D))
    O-->>C: emit values
    C->>C: handle emissions

    Note over C: Component destroyed (navigation/unmount)
    C->>D: destroy$.next()
    D->>O: takeUntil triggers unsubscription
    C->>D: destroy$.complete()

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Pay special attention to NotificationsTrayComponent for multiple concurrent subscriptions and forkJoin usage.
  • Confirm every subscription site in the changed files uses takeUntil(this.destroy$) (including nested/valueChanges and API calls).
  • Verify destroy$ is private and is .next() then .complete() in ngOnDestroy().

Suggested reviewers

  • alberto-art3ch

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: fixing memory leaks in multiple components by managing subscriptions, which directly aligns with all file modifications implementing OnDestroy and takeUntil patterns.
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

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.

coderabbitai[bot] avatar Nov 21 '25 19:11 coderabbitai[bot]