Skip to content

fix(core): unregister onDestroy in toSignal.#61514

Closed
JeanMeche wants to merge 1 commit intoangular:mainfrom
JeanMeche:fix/toSignal-onDestroy
Closed

fix(core): unregister onDestroy in toSignal.#61514
JeanMeche wants to merge 1 commit intoangular:mainfrom
JeanMeche:fix/toSignal-onDestroy

Conversation

@JeanMeche
Copy link
Copy Markdown
Member

We cleanup if the observable completes before the onDestroy fires.

fixes #61511

We cleanup if the observable completes before the onDestroy fires.

fixes angular#61511
@pullapprove pullapprove bot requested a review from atscott May 20, 2025 13:45
@angular-robot angular-robot bot added the area: core Issues related to the framework runtime label May 20, 2025
@ngbot ngbot bot added this to the Backlog milestone May 20, 2025
@JeanMeche JeanMeche removed the request for review from atscott May 20, 2025 13:54
@JeanMeche JeanMeche 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 20, 2025
thePunderWoman pushed a commit that referenced this pull request May 20, 2025
We cleanup if the observable completes before the onDestroy fires.

fixes #61511

PR Close #61514
thePunderWoman pushed a commit that referenced this pull request May 20, 2025
We cleanup if the observable completes before the onDestroy fires.

fixes #61511

PR Close #61514
@thePunderWoman
Copy link
Copy Markdown
Contributor

This PR was merged into the repository by commit 1148b7e.

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

@JeanMeche JeanMeche deleted the fix/toSignal-onDestroy branch May 20, 2025 16:19
@@ -163,6 +165,9 @@ export function toSignal<T, U = undefined>(
error: (error) => {
state.set({kind: StateKind.Error, error});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

we should also unregister the onDestroy callback here, because complete is not called if the Observable terminates with error

@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 Jul 15, 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.

toSignal: remove cleanup callback from DestroyRef when Observable terminates

4 participants