Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@arturovt
Copy link
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
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
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
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
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
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
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
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

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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.