Skip to content

fix(core): control not recognized when input has directive injecting ViewContainerRef#64368

Closed
crisbeto wants to merge 1 commit intoangular:mainfrom
crisbeto:64362/control-vcr
Closed

fix(core): control not recognized when input has directive injecting ViewContainerRef#64368
crisbeto wants to merge 1 commit intoangular:mainfrom
crisbeto:64362/control-vcr

Conversation

@crisbeto
Copy link
Copy Markdown
Member

@crisbeto crisbeto commented Oct 12, 2025 •
edited
Loading

When a directive injects a ViewContainerRef, the runtime inserts a container that was throwing off the logic that recognizes native controls.

These changes switch to check if the node is a native control through the TNode. This also makes it a bit less prone to breaking during SSR.

Fixes #64362.

@crisbeto crisbeto requested a review from leonsenft October 12, 2025 01:59
@crisbeto crisbeto added action: review The PR is still awaiting reviews from at least one requested reviewer target: minor This PR is targeted for the next minor release labels Oct 12, 2025
@angular-robot angular-robot bot added the area: core Issues related to the framework runtime label Oct 12, 2025
@ngbot ngbot bot added this to the Backlog milestone Oct 12, 2025
@JeanMeche JeanMeche added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Oct 13, 2025
@ngbot
Copy link
Copy Markdown

ngbot bot commented Oct 13, 2025

I see that you just added the action: merge label, but the following checks are still failing:
    failure conflicts with base branch "main"
    pending status "google-internal-tests" is pending

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken main, please try rebasing to main and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

@JeanMeche
Copy link
Copy Markdown
Member

JeanMeche commented Oct 13, 2025

(rebased to solve the conflict)

@AndrewKushnir
Copy link
Copy Markdown
Contributor

AndrewKushnir commented Oct 13, 2025

It looks like the test CI job is "red" in this PR, adding the "cleanup" label for now.

@AndrewKushnir AndrewKushnir added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Oct 13, 2025
@AndrewKushnir AndrewKushnir removed the action: merge The PR is ready for merge by the caretaker label Oct 13, 2025
@JeanMeche JeanMeche force-pushed the 64362/control-vcr branch 2 times, most recently from 5f60bfa to e339787 Compare October 13, 2025 20:24
leonsenft
leonsenft reviewed Oct 13, 2025
View reviewed changes
@JeanMeche JeanMeche force-pushed the 64362/control-vcr branch 3 times, most recently from e25de25 to 4eb01eb Compare October 13, 2025 21:36
leonsenft
leonsenft approved these changes Oct 13, 2025
View reviewed changes
@crisbeto
fix(core): control not recognized when input has directive injecting …
a684c16 
…ViewContainerRef

When a directive injects a `ViewContainerRef`, the runtime inserts a container that was throwing off the logic that recognizes native controls.

These changes switch to check if the node is a native control through the `TNode`. This also makes it a bit less prone to breaking during SSR.

Fixes angular#64362.
@crisbeto
Copy link
Copy Markdown
Member Author

crisbeto commented Oct 13, 2025

Rebased and added an early exit for the loop that checks the type of the input.

@crisbeto crisbeto 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 Oct 13, 2025
@crisbeto crisbeto added merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels Oct 13, 2025
@AndrewKushnir
Copy link
Copy Markdown
Contributor

AndrewKushnir commented Oct 14, 2025

This PR was merged into the repository. The changes were merged into the following branches:

@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot bot commented Nov 14, 2025

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

Reviewers

@JeanMeche JeanMeche JeanMeche left review comments

@leonsenft leonsenft leonsenft 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: minor This PR is targeted for the next minor release

Projects

None yet

Milestone

Backlog

4 participants

@crisbeto @JeanMeche @AndrewKushnir @leonsenft