Skip to content

perf(core): avoid changes Observable creation on QueryList#53498

Closed
pkozlowski-opensource wants to merge 1 commit intoangular:mainfrom
pkozlowski-opensource:queries_cleanup
Closed

perf(core): avoid changes Observable creation on QueryList#53498
pkozlowski-opensource wants to merge 1 commit intoangular:mainfrom
pkozlowski-opensource:queries_cleanup

Conversation

@pkozlowski-opensource
Copy link
Copy Markdown
Member

The changes Observable (impl: EventEmitter) on the QueryList is initalized lazy - it is created only if someone calls a geter to get a hand on its instance. But the destroy method was calling this getter thus creating a new Observable even if no one subscribed to it.

This commit changes the destroy logic to skip creation of an EventEmitter if it wasn't initialized.

@pkozlowski-opensource pkozlowski-opensource 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 labels Dec 11, 2023
@ngbot ngbot bot modified the milestone: Backlog Dec 11, 2023
Copy link
Copy Markdown
Contributor

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

The changes Observable (impl: EventEmitter) on the QueryList is initalized
lazy - it is created only if someone calls a geter to get a hand on its
instance. But the destroy method was calling this getter thus creating
a new Observable even if no one subscribed to it.

This commit changes the destroy logic to skip creation of an EventEmitter
if it wasn't initialized.
@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 Dec 11, 2023
@alxhub
Copy link
Copy Markdown
Member

alxhub commented Dec 12, 2023

This PR was merged into the repository by commit 1fc5442.

alxhub pushed a commit that referenced this pull request Dec 12, 2023
The changes Observable (impl: EventEmitter) on the QueryList is initalized
lazy - it is created only if someone calls a geter to get a hand on its
instance. But the destroy method was calling this getter thus creating
a new Observable even if no one subscribed to it.

This commit changes the destroy logic to skip creation of an EventEmitter
if it wasn't initialized.

PR Close #53498
@alxhub alxhub closed this in 1fc5442 Dec 12, 2023
@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 Jan 12, 2024
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
…3498)

The changes Observable (impl: EventEmitter) on the QueryList is initalized
lazy - it is created only if someone calls a geter to get a hand on its
instance. But the destroy method was calling this getter thus creating
a new Observable even if no one subscribed to it.

This commit changes the destroy logic to skip creation of an EventEmitter
if it wasn't initialized.

PR Close angular#53498
rlmestre pushed a commit to rlmestre/angular that referenced this pull request Jan 26, 2024
…3498)

The changes Observable (impl: EventEmitter) on the QueryList is initalized
lazy - it is created only if someone calls a geter to get a hand on its
instance. But the destroy method was calling this getter thus creating
a new Observable even if no one subscribed to it.

This commit changes the destroy logic to skip creation of an EventEmitter
if it wasn't initialized.

PR Close angular#53498
amilamen pushed a commit to amilamen/angular that referenced this pull request Jan 26, 2024
…3498)

The changes Observable (impl: EventEmitter) on the QueryList is initalized
lazy - it is created only if someone calls a geter to get a hand on its
instance. But the destroy method was calling this getter thus creating
a new Observable even if no one subscribed to it.

This commit changes the destroy logic to skip creation of an EventEmitter
if it wasn't initialized.

PR Close angular#53498
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 target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants