Skip to content

refactor: make LView[Injector] non nullable #58805

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
kirjs wants to merge 1 commit into from
Closed

refactor: make LView[Injector] non nullable #58805

kirjs wants to merge 1 commit into from

Conversation

@kirjs
Copy link
Contributor

@kirjs kirjs commented Nov 21, 2024

Currently, the LView[INJECTOR] typings include null, when in practice we don't have cases when that may happen. That requires using non-null assertions in the code.

This PR makes LView[INJECTOR] nonnullable and drops all the assertions

@pullapprove pullapprove bot requested a review from AndrewKushnir November 21, 2024 21:33
AndrewKushnir
AndrewKushnir approved these changes Nov 21, 2024
View reviewed changes
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

LGTM, just a minor proposal to update a comment to indicate that the injector is now non-optional 👍

@AndrewKushnir AndrewKushnir added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews area: core Issues related to the framework runtime target: patch This PR is targeted for the next patch release labels Nov 21, 2024
@ngbot ngbot bot modified the milestone: Backlog Nov 21, 2024
@kirjs kirjs force-pushed the refactor-make-lview-injector-nonnullable branch from 9a759a1 to 4005ca4 Compare November 22, 2024 14:57
@angular-robot angular-robot bot removed the area: core Issues related to the framework runtime label Nov 22, 2024
@ngbot ngbot bot removed this from the Backlog milestone Nov 22, 2024
@thePunderWoman thePunderWoman added the area: core Issues related to the framework runtime label Nov 22, 2024
@ngbot ngbot bot added this to the Backlog milestone Nov 22, 2024
@JeanMeche
Copy link
Member

JeanMeche commented Nov 22, 2024

The lint job if failing because the commit message lacks a commit body of minimum 20chars.

@kirjs kirjs force-pushed the refactor-make-lview-injector-nonnullable branch from 4005ca4 to 4e80ec4 Compare November 22, 2024 16:17
@angular-robot angular-robot bot removed the area: core Issues related to the framework runtime label Nov 22, 2024
@ngbot ngbot bot removed this from the Backlog milestone Nov 22, 2024
@thePunderWoman
Copy link
Contributor

thePunderWoman commented Nov 22, 2024

This should also have a scope in the commit. It should be refactor(core): make LView[Injector] non nullable. Otherwise it'll continue to have the area label removed automatically.

@thePunderWoman thePunderWoman added the area: core Issues related to the framework runtime label Nov 22, 2024
@ngbot ngbot bot added this to the Backlog milestone Nov 22, 2024
@kirjs kirjs force-pushed the refactor-make-lview-injector-nonnullable branch from 4e80ec4 to 8ec880e Compare November 22, 2024 17:34
@angular-robot angular-robot bot removed the area: core Issues related to the framework runtime label Nov 22, 2024
@ngbot ngbot bot removed this from the Backlog milestone Nov 22, 2024
@NothingEverHappens
refactor(core): make lView[Injector] non-nullable
162273c 
There actually no cases, where it can be null. Also update all usages.
@kirjs kirjs force-pushed the refactor-make-lview-injector-nonnullable branch from 8ec880e to 162273c Compare November 22, 2024 20:50
@angular-robot angular-robot bot added the area: core Issues related to the framework runtime label Nov 22, 2024
@ngbot ngbot bot added this to the Backlog milestone Nov 22, 2024
@thePunderWoman thePunderWoman added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Nov 22, 2024
thePunderWoman pushed a commit that referenced this pull request Nov 25, 2024
@NothingEverHappens @thePunderWoman
refactor(core): make lView[Injector] non-nullable (#58805)
70547eb 
There actually no cases, where it can be null. Also update all usages.

PR Close #58805
@thePunderWoman
Copy link
Contributor

thePunderWoman commented Nov 25, 2024

This PR was merged into the repository by commit 15f0716.

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

@angular-automatic-lock-bot
Copy link

angular-automatic-lock-bot bot commented Dec 26, 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 Dec 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@AndrewKushnir AndrewKushnir AndrewKushnir approved these changes

Assignees

No one assigned

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.

5 participants

@kirjs @JeanMeche @thePunderWoman @AndrewKushnir @NothingEverHappens