Skip to content

fix(core): ensure all initializer functions run in an injection context#54761

Closed
crisbeto wants to merge 1 commit intoangular:mainfrom
crisbeto:injection-context-initializers
Closed

fix(core): ensure all initializer functions run in an injection context#54761
crisbeto wants to merge 1 commit intoangular:mainfrom
crisbeto:injection-context-initializers

Conversation

@crisbeto
Copy link
Member

@crisbeto crisbeto commented Mar 8, 2024

Ensures that all of the functions intended to be run in initializers are in an injection context. This is a stop-gap until we have a compiler diagnostic for it.

@crisbeto crisbeto added action: review The PR is still awaiting reviews from at least one requested reviewer target: rc This PR is targeted for the next release-candidate labels Mar 8, 2024
@crisbeto crisbeto requested a review from devversion March 8, 2024 10:05
@crisbeto crisbeto force-pushed the injection-context-initializers branch from fbfb314 to 493b515 Compare March 8, 2024 12:18
@crisbeto crisbeto marked this pull request as ready for review March 8, 2024 12:28
Copy link
Member Author

Choose a reason for hiding this comment

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

I had to rework these tests, because they were instantiating components manually.

Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

LGTM. Only would optionally suggest TestBed.runInInjectionContext for shorter tests

Copy link
Member

Choose a reason for hiding this comment

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

TestBed.runInInjectionContext(() => input(0)) also works and is shorter.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, I'll change the input tests but will keep the query tests as they are since they're kinda tied to a view.

Ensures that all of the functions intended to be run in initializers are in an injection context. This is a stop-gap until we have a compiler diagnostic for it.
@crisbeto crisbeto force-pushed the injection-context-initializers branch from 493b515 to 31bfdb3 Compare March 8, 2024 13:23
@crisbeto crisbeto added state: blocked and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Mar 8, 2024
@crisbeto
Copy link
Member Author

crisbeto commented Mar 8, 2024

Blocked on reworking internal targets that are using the functions incorrectly.

@crisbeto
Copy link
Member Author

Passing TGP, excluding some flakes and after I land a separate cleanup CL.

@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker and removed state: blocked labels Mar 12, 2024
@atscott
Copy link
Contributor

atscott commented Mar 12, 2024

This PR was merged into the repository by commit 018f826.

atscott pushed a commit that referenced this pull request Mar 12, 2024
…xt (#54761)

Ensures that all of the functions intended to be run in initializers are in an injection context. This is a stop-gap until we have a compiler diagnostic for it.

PR Close #54761
@atscott atscott closed this in 018f826 Mar 12, 2024
@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 Apr 12, 2024
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 target: rc This PR is targeted for the next release-candidate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants