Skip to content

fix(core): ensure that standalone components get correct injector instances#50954

Closed
AndrewKushnir wants to merge 1 commit intoangular:mainfrom
AndrewKushnir:standalone_di
Closed

fix(core): ensure that standalone components get correct injector instances#50954
AndrewKushnir wants to merge 1 commit intoangular:mainfrom
AndrewKushnir:standalone_di

Conversation

@AndrewKushnir
Copy link
Copy Markdown
Contributor

Prior to this change, we've used componentDef.id as a key in a Map that acts as a cache to avoid re-creating injector instances for standalone components. In v16, the logic that generates the id has changed from an auto-incremental to a generation based on metadata. If multiple components have similar metadata, their ids might overlap.

This commit updates the logic to stop using componentDef.id as a key and instead, use the componentDef itself. This would ensure that we always have a correct instance of an injector associated with a standalone component instance.

Resolves #50724.

PR Type

What kind of change does this PR introduce?

  • Bugfix

Does this PR introduce a breaking change?

  • Yes
  • No

…tances

Prior to this change, we've used `componentDef.id` as a key in a Map that acts as a cache to avoid re-creating injector instances for standalone components. In v16, the logic that generates the id has changed from an auto-incremental to a generation based on metadata. If multiple components have similar metadata, their ids might overlap.

This commit updates the logic to stop using `componentDef.id` as a key and instead, use the `componentDef` itself. This would ensure that we always have a correct instance of an injector associated with a standalone component instance.

Resolves angular#50724.
@AndrewKushnir AndrewKushnir added action: review The PR is still awaiting reviews from at least one requested reviewer area: core Issues related to the framework runtime target: patch This PR is targeted for the next patch release action: global presubmit The PR is in need of a google3 global presubmit cross-cutting: standalone Issues related to the NgModule-less world labels Jul 6, 2023
@ngbot ngbot bot modified the milestone: Backlog Jul 6, 2023
@AndrewKushnir
Copy link
Copy Markdown
Contributor Author

AndrewKushnir commented Jul 6, 2023

Exploratory TGP.

@crisbeto
Copy link
Copy Markdown
Member

crisbeto commented Jul 6, 2023

We were talking about this during triage yesterday with @pkozlowski-opensource and @JoostK. I initially introduced it with #46093 to try and reduce the chance of memory leaks. I don't mind going back to tracking it via a reference to the definition itself, but the fact that component IDs aren't actually unique may be a problem in the future.

@pkozlowski-opensource
Copy link
Copy Markdown
Member

Yep, the fact that component ids are not unique might cause issues in other places. Let's discuss before merging.

@pkozlowski-opensource pkozlowski-opensource 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 Jul 6, 2023
@AndrewKushnir AndrewKushnir removed the action: global presubmit The PR is in need of a google3 global presubmit label Jul 6, 2023
@AndrewKushnir
Copy link
Copy Markdown
Contributor Author

Caretaker note: TGP is "green", this PR is ready for merge.

@AndrewKushnir AndrewKushnir added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Jul 7, 2023
@dylhunn
Copy link
Copy Markdown
Contributor

dylhunn commented Jul 10, 2023

This PR was merged into the repository by commit 031b599.

dylhunn pushed a commit that referenced this pull request Jul 10, 2023
…tances (#50954)

Prior to this change, we've used `componentDef.id` as a key in a Map that acts as a cache to avoid re-creating injector instances for standalone components. In v16, the logic that generates the id has changed from an auto-incremental to a generation based on metadata. If multiple components have similar metadata, their ids might overlap.

This commit updates the logic to stop using `componentDef.id` as a key and instead, use the `componentDef` itself. This would ensure that we always have a correct instance of an injector associated with a standalone component instance.

Resolves #50724.

PR Close #50954
@dylhunn dylhunn closed this in 031b599 Jul 10, 2023
sunilbaba pushed a commit to sunilbaba/angular that referenced this pull request Jul 26, 2023
…tances (angular#50954)

Prior to this change, we've used `componentDef.id` as a key in a Map that acts as a cache to avoid re-creating injector instances for standalone components. In v16, the logic that generates the id has changed from an auto-incremental to a generation based on metadata. If multiple components have similar metadata, their ids might overlap.

This commit updates the logic to stop using `componentDef.id` as a key and instead, use the `componentDef` itself. This would ensure that we always have a correct instance of an injector associated with a standalone component instance.

Resolves angular#50724.

PR Close angular#50954
@angular-automatic-lock-bot
Copy link
Copy Markdown

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 Aug 10, 2023
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
…tances (angular#50954)

Prior to this change, we've used `componentDef.id` as a key in a Map that acts as a cache to avoid re-creating injector instances for standalone components. In v16, the logic that generates the id has changed from an auto-incremental to a generation based on metadata. If multiple components have similar metadata, their ids might overlap.

This commit updates the logic to stop using `componentDef.id` as a key and instead, use the `componentDef` itself. This would ensure that we always have a correct instance of an injector associated with a standalone component instance.

Resolves angular#50724.

PR Close angular#50954
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 area: core Issues related to the framework runtime cross-cutting: standalone Issues related to the NgModule-less world merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Issue with assigning lazy loaded standalone components environment injectors

4 participants