Skip to content

fix(core): signals should be tracked when embeddedViewRef.detectChanges is called #55719

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
atscott wants to merge 1 commit into from
Closed

fix(core): signals should be tracked when embeddedViewRef.detectChanges is called #55719

atscott wants to merge 1 commit into from

Conversation

@atscott
Copy link
Contributor

@atscott atscott commented May 7, 2024

This commit fixes an issue where signals in embedded views are not tracked if they are refreshed with EmbeddedViewRef.detectChanges directly. We had previously assumed that embedded views were always refreshed along with their hosts.

@atscott atscott added the area: core Issues related to the framework runtime label May 7, 2024
@ngbot ngbot bot added this to the Backlog milestone May 7, 2024
@atscott atscott added the target: patch This PR is targeted for the next patch release label May 7, 2024
@atscott atscott requested a review from alxhub May 7, 2024 22:38
@atscott atscott force-pushed the reactiveconsumerembeddedviews branch from 525f882 to 9342ac6 Compare May 7, 2024 22:38
@atscott atscott force-pushed the reactiveconsumerembeddedviews branch from 9342ac6 to 1903d99 Compare May 20, 2024 21:55
@atscott atscott marked this pull request as ready for review May 20, 2024 21:55
alxhub
alxhub approved these changes May 30, 2024
View reviewed changes
packages/core/src/render3/instructions/change_detection.ts Outdated
Copy link
Member

@alxhub alxhub May 30, 2024

Choose a reason for hiding this comment

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

What do you think about having a debug assertion here that the consumer is a temporary allocated one?

@atscott atscott force-pushed the reactiveconsumerembeddedviews branch 2 times, most recently from bfe7a55 to c29b3ef Compare May 30, 2024 22:00
@atscott
fix(core): signals should be tracked when embeddedViewRef.detectChang…
264393e 
…es is called

This commit fixes an issue where signals in embedded views are not
tracked if they are refreshed with `EmbeddedViewRef.detectChanges`
directly. We had previously assumed that embedded views were always
refreshed along with their hosts.
@atscott atscott force-pushed the reactiveconsumerembeddedviews branch from c29b3ef to 264393e Compare June 4, 2024 17:07
@atscott atscott removed the request for review from pkozlowski-opensource June 10, 2024 23:47
@atscott atscott added the action: merge The PR is ready for merge by the caretaker label Jun 10, 2024
@atscott atscott self-assigned this Jun 11, 2024
@AndrewKushnir
Copy link
Contributor

AndrewKushnir commented Jun 11, 2024

This PR was merged into the repository by commit 4c7d5d8.

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

AndrewKushnir pushed a commit that referenced this pull request Jun 11, 2024
@atscott @AndrewKushnir
fix(core): signals should be tracked when embeddedViewRef.detectChang…
625ca3e 
…es is called (#55719)

This commit fixes an issue where signals in embedded views are not
tracked if they are refreshed with `EmbeddedViewRef.detectChanges`
directly. We had previously assumed that embedded views were always
refreshed along with their hosts.

PR Close #55719
@crokenx crokenx mentioned this pull request Jul 3, 2024
Open
@sixcorners sixcorners mentioned this pull request Jul 4, 2024
Closed
@hantsy hantsy mentioned this pull request Jul 4, 2024
Closed
@juanponton5 juanponton5 mentioned this pull request Jul 4, 2024
Open
@ambrose40 ambrose40 mentioned this pull request Jul 4, 2024
Open
This was referenced Jul 4, 2024
Open
Open
Open
@MrDari MrDari mentioned this pull request Jul 5, 2024
Open
@hantsy hantsy mentioned this pull request Jul 10, 2024
Closed
@angular-automatic-lock-bot
Copy link

angular-automatic-lock-bot bot commented Jul 12, 2024

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 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@alxhub alxhub alxhub approved these changes

Assignees

@atscott atscott

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

Milestone

Backlog

Development

Successfully merging this pull request may close these issues.

3 participants

@atscott @AndrewKushnir @alxhub