Skip to content

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

Closed
atscott wants to merge 1 commit intoangular:mainfrom
atscott:removeExpressionChangedSafeguards
Closed

test(core): Add test to assert current behavior for setting signals in afterRender#52475
atscott wants to merge 1 commit intoangular:mainfrom
atscott:removeExpressionChangedSafeguards

Conversation

@atscott
Copy link
Copy Markdown
Contributor

@atscott atscott commented Oct 31, 2023 •
edited
Loading

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
Copy Markdown
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
Copy Markdown
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
@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot bot commented Dec 9, 2023

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.

3 participants

@atscott @JeanMeche @alxhub