Skip to content
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

test(core): Add test to assert current behavior for setting signals in afterRender #52475

Closed
wants to merge 1 commit into from
Closed

test(core): Add test to assert current behavior for setting signals in afterRender #52475

wants to merge 1 commit into from

Conversation

atscott
Copy link
Contributor

@atscott atscott commented Oct 31, 2023 •
edited

We do not yet handle running change detection again if afterRender
hooks write to signals.

@atscott atscott added the target: rc This PR is targeted for the next release-candidate label Oct 31, 2023
@atscott atscott force-pushed the removeExpressionChangedSafeguards branch from 65d00ed to c85ee47 Compare October 31, 2023 21:28
@atscott atscott added this to the v17-final milestone Oct 31, 2023
@ngbot ngbot bot removed this from the v17-final milestone Oct 31, 2023
@atscott atscott force-pushed the removeExpressionChangedSafeguards branch 2 times, most recently from 7b993fa to 701d150 Compare October 31, 2023 22:02
@atscott atscott mentioned this pull request Oct 31, 2023
Closed
@atscott atscott added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels Oct 31, 2023
@atscott
Copy link
Contributor Author

atscott commented Oct 31, 2023

caretaker note: aio test failures are pre-existing

@atscott atscott 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 merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels Nov 6, 2023
@atscott
test(core): Add test to ensure writing to signals in afterRender hook…
8c608e2 
…s throws error

We do not yet handle running change detection again if `afterRender`
hooks write to signals.
@atscott atscott force-pushed the removeExpressionChangedSafeguards branch from 701d150 to 8c608e2 Compare November 7, 2023 21:57
@atscott atscott added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews target: rc This PR is targeted for the next release-candidate labels Nov 7, 2023
@atscott atscott changed the title refactor(core): Remove warning about signal set during template execution test(core): Add test to assert current behavior for setting signals in afterRender Nov 7, 2023
@atscott
Copy link
Contributor Author

atscott commented Nov 8, 2023

This PR was merged into the repository by commit 8592585.

atscott added a commit that referenced this pull request Nov 8, 2023
@atscott
test(core): Add test to ensure writing to signals in afterRender hook…
e1c0493 
…s throws error (#52475)

We do not yet handle running change detection again if `afterRender`
hooks write to signals.

PR Close #52475
@atscott atscott closed this in 8592585 Nov 8, 2023
tbondwilkinson pushed a commit to tbondwilkinson/angular that referenced this pull request Dec 6, 2023
@atscott @tbondwilkinson
test(core): Add test to ensure writing to signals in afterRender hook…
8fb8f3e 
…s throws error (angular#52475)

We do not yet handle running change detection again if `afterRender`
hooks write to signals.

PR Close angular#52475
@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 Dec 9, 2023
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
@atscott @ChellappanRajan
test(core): Add test to ensure writing to signals in afterRender hook…
b03010d 
…s throws error (angular#52475)

We do not yet handle running change detection again if `afterRender`
hooks write to signals.

PR Close angular#52475
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@JeanMeche JeanMeche JeanMeche left review comments

@alxhub alxhub alxhub approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@atscott @JeanMeche @alxhub