Web 428 memory leaks in multiple components due to unmanaged subscriptions
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.
[!NOTE]
.coderabbit.yamlhas unrecognized propertiesCodeRabbit 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$isprivateand is.next()then.complete()inngOnDestroy().
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.
Comment @coderabbitai help to get the list of available commands and usage tips.