Skip to content

fix(core): cleanup testability subscriptions#61261

Closed
arturovt wants to merge 1 commit intoangular:mainfrom
arturovt:fix/core-testability-subscriptions
Closed

fix(core): cleanup testability subscriptions#61261
arturovt wants to merge 1 commit intoangular:mainfrom
arturovt:fix/core-testability-subscriptions

Conversation

@arturovt
Copy link
Copy Markdown
Contributor

This commit prevents leaking memory when the application is destroyed and subscriptions are still alive.

@pullapprove pullapprove bot requested a review from devversion May 11, 2025 10:59
@angular-robot angular-robot bot added the area: core Issues related to the framework runtime label May 11, 2025
@ngbot ngbot bot added this to the Backlog milestone May 11, 2025
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wouldn't takeUntilDetroyed be more adequate ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It becomes a circular dependency (core depends on rxjs-interop and vice versa).

@devversion devversion requested a review from atscott May 12, 2025 10:59
@devversion
Copy link
Copy Markdown
Member

@thePunderWoman thePunderWoman added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels May 16, 2025
@thePunderWoman
Copy link
Copy Markdown
Contributor

@arturovt Looks like there's a circular dependency added here.

@thePunderWoman thePunderWoman added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: merge The PR is ready for merge by the caretaker labels May 16, 2025
@arturovt arturovt force-pushed the fix/core-testability-subscriptions branch 2 times, most recently from c1dff20 to bc57aff Compare May 16, 2025 13:26
@JeanMeche JeanMeche closed this May 20, 2025
@JeanMeche JeanMeche reopened this May 20, 2025
@JeanMeche JeanMeche force-pushed the fix/core-testability-subscriptions branch from bc57aff to 5cdb1aa Compare May 20, 2025 23:14
@JeanMeche JeanMeche added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels May 20, 2025
@thePunderWoman thePunderWoman force-pushed the fix/core-testability-subscriptions branch from 5cdb1aa to f987f88 Compare May 21, 2025 10:09
@thePunderWoman thePunderWoman added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: merge The PR is ready for merge by the caretaker labels May 21, 2025
@thePunderWoman
Copy link
Copy Markdown
Contributor

Looks like a broken integration test.

@JeanMeche JeanMeche force-pushed the fix/core-testability-subscriptions branch from f987f88 to c747d54 Compare May 21, 2025 10:29
@pullapprove pullapprove bot requested a review from kirjs May 21, 2025 10:29
This commit prevents leaking memory when the application is destroyed and subscriptions are still alive.
@JeanMeche JeanMeche force-pushed the fix/core-testability-subscriptions branch from c747d54 to 278154d Compare May 21, 2025 11:09
@thePunderWoman thePunderWoman added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels May 21, 2025
Copy link
Copy Markdown
Contributor

@thePunderWoman thePunderWoman left a comment

Choose a reason for hiding this comment

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

reviewed-for: size-tracking

thePunderWoman pushed a commit that referenced this pull request May 21, 2025
This commit prevents leaking memory when the application is destroyed and subscriptions are still alive.

PR Close #61261
thePunderWoman pushed a commit that referenced this pull request May 21, 2025
This commit prevents leaking memory when the application is destroyed and subscriptions are still alive.

PR Close #61261
@thePunderWoman
Copy link
Copy Markdown
Contributor

This PR was merged into the repository by commit 6bf6dbc.

The changes were merged into the following branches: main, 19.2.x, 20.0.x

@arturovt arturovt deleted the fix/core-testability-subscriptions branch May 21, 2025 12:06
@angular-automatic-lock-bot
Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jun 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants