Skip to content

fix(core): Ensure that a destroyed effect never runs#59415

Closed
JeanMeche wants to merge 1 commit intoangular:mainfrom
JeanMeche:fix/destroyed-effect
Closed

fix(core): Ensure that a destroyed effect never runs#59415
JeanMeche wants to merge 1 commit intoangular:mainfrom
JeanMeche:fix/destroyed-effect

Conversation

@JeanMeche
Copy link
Copy Markdown
Member

Prior to this change, a scheduled root effect, even if destroyed instantly, would still run at least once.

This commit fixes this.

fixes #59410

@JeanMeche JeanMeche requested a review from alxhub January 7, 2025 20:46
@angular-robot angular-robot bot added the area: core Issues related to the framework runtime label Jan 7, 2025
@JeanMeche JeanMeche changed the title fix(core): Ensure that a destroyed effect never run. fix(core): Ensure that a destroyed effect never runs Jan 7, 2025
@ngbot ngbot bot added this to the Backlog milestone Jan 7, 2025
Prior to this change, a scheduled root effect, even if destroyed instantly, would still run at least once.

This commit fixes this.

fixes angular#59410
@JeanMeche JeanMeche force-pushed the fix/destroyed-effect branch from 56de843 to 9fb5898 Compare January 7, 2025 21:15
@angular-robot angular-robot bot added area: core Issues related to the framework runtime and removed area: core Issues related to the framework runtime labels Jan 7, 2025
@ngbot ngbot bot modified the milestone: Backlog Jan 7, 2025
Copy link
Copy Markdown
Member

@alxhub alxhub 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

@pullapprove pullapprove bot requested a review from thePunderWoman January 8, 2025 16:00
@thePunderWoman thePunderWoman added the target: patch This PR is targeted for the next patch release label Jan 8, 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.

LGTM

@thePunderWoman thePunderWoman added the action: merge The PR is ready for merge by the caretaker label Jan 8, 2025
@thePunderWoman
Copy link
Copy Markdown
Contributor

Caretaker note: this is safe to merge. Failing tests are flakes.

@thePunderWoman thePunderWoman added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Jan 8, 2025
@thePunderWoman
Copy link
Copy Markdown
Contributor

This PR was merged into the repository by commit 5c0d688.

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

thePunderWoman pushed a commit that referenced this pull request Jan 8, 2025
Prior to this change, a scheduled root effect, even if destroyed instantly, would still run at least once.

This commit fixes this.

fixes #59410

PR Close #59415
kirjs added a commit to kirjs/angular that referenced this pull request Jan 9, 2025
kirjs added a commit to kirjs/angular that referenced this pull request Jan 15, 2025
PrajaktaB27 pushed a commit to PrajaktaB27/angular that referenced this pull request Feb 7, 2025
Prior to this change, a scheduled root effect, even if destroyed instantly, would still run at least once.

This commit fixes this.

fixes angular#59410

PR Close angular#59415
@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 Feb 8, 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 merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EffectRef.destroy has no impact

4 participants