Skip to content

WEB-643 fix(clients): add takeUntil pattern to prevent memory leaks i… - #3065

Merged
IOhacker merged 1 commit into
openMF:devfrom
shubhamkumar9199:fix/client-components-memory-leak
Jan 31, 2026
Merged

WEB-643 fix(clients): add takeUntil pattern to prevent memory leaks i…#3065
IOhacker merged 1 commit into
openMF:devfrom
shubhamkumar9199:fix/client-components-memory-leak

Conversation

@shubhamkumar9199

@shubhamkumar9199 shubhamkumar9199 commented Jan 31, 2026

Copy link
Copy Markdown
Contributor

…n client-general-step component

This PR fixes memory leaks caused by unsubscribed RxJS
WEB-643

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

  • If you have multiple commits please combine them into one commit by squashing them.

  • Read and understood the contribution guidelines at web-app/.github/CONTRIBUTING.md.

Summary by CodeRabbit

  • Refactor

    • Improved component lifecycle and subscription handling to ensure reliable cleanup and reduce memory leaks.
  • Bug Fixes

    • Fixed inconsistent form behavior when switching offices or toggling options, improving template and staff option updates and preventing stale data from persisting.

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

@coderabbitai

coderabbitai Bot commented Jan 31, 2026

Copy link
Copy Markdown

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

Reworked the component to implement OnDestroy and add a private destroy$ Subject; converted several valueChanges subscriptions to pipe(..., takeUntil(this.destroy$)), added ngOnDestroy() to emit/complete the subject, and enhanced officeId handling with filter/switchMap for template/staff updates.

Changes

Cohort / File(s) Summary
Client General Step Component
src/app/clients/client-stepper/client-general-step/client-general-step.component.ts
Added OnDestroy to the component, introduced private destroy$ = new Subject<void>(), converted multiple valueChanges subscriptions to use pipe(..., takeUntil(this.destroy$)) (legalFormId, active, addSavings, officeId), enhanced officeId subscription with filter and switchMap to fetch client templates and update staffOptions, added ngOnDestroy() to emit/complete destroy$, and updated imports to include Subject, filter, switchMap, and takeUntil.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • alberto-art3ch
🚥 Pre-merge checks | ✅ 3
✅ 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 summarizes the main change: adding the takeUntil pattern to prevent memory leaks in the client-general-step component, which aligns with the substantial refactoring shown in the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In
`@src/app/clients/client-stepper/client-general-step/client-general-step.component.ts`:
- Around line 229-239: The current nested subscription on
createClientForm.get('officeId').valueChanges causes multiple in-flight
requests; refactor to pipe valueChanges through switchMap to
clientService.getClientWithOfficeTemplate(officeId) and then subscribe (still
terminating with takeUntil(this.destroy$)) so previous HTTP calls are cancelled
when officeId changes; update imports to include switchMap and replace the inner
subscribe that sets this.staffOptions with a single subscription handling the
response from switchMap.

@shubhamkumar9199
shubhamkumar9199 force-pushed the fix/client-components-memory-leak branch from 4d7b05a to 1e3dc6a Compare January 31, 2026 19:33
@IOhacker

Copy link
Copy Markdown
Contributor

LGTM

@IOhacker IOhacker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@IOhacker
IOhacker merged commit 163ad13 into openMF:dev Jan 31, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants