Skip to content
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.

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

fix(core): Multiple subscribers to ApplicationRef.isStable should all… #53541

Closed
wants to merge 1 commit into from

Conversation

atscott
Copy link
Contributor

@atscott atscott commented Dec 12, 2023

… see values

The behavior of ApplicationRef.isStable changed in 16.1 due to 28c68f7. This change added a share to the isStable observable, which prevents additional subscribers from getting a value until a new one emits. One solution to the problem would be shareReplay(1). However, that would increase the bundle size since we do not use shareReplay elsewhere. Instead, we don't even really need to share the observable.

The Observable available in ApplicationRef.isStable before the above commit was the zone stable observable, without a share. The new behavior adds only an additional observable to the stream, hasPendingTasks (a BehaviorSubject). The observables in this stream are not expensive to subscribe to. The only one with side effects is the isStable (because it subscirbes to onStable), but that one already has the share operator on it. Omitting the share in ApplicationRef also means that applications on zoneless will not have to pay the cost of the operator when we make zones optional because the zone stable observable is the only place we use it.

… see values

The behavior of `ApplicationRef.isStable` changed in 16.1 due to
angular@28c68f7.
This change added a `share` to the `isStable` observable, which prevents
additional subscribers from getting a value until a new one emits. One
solution to the problem would be `shareReplay(1)`. However, that would
increase the bundle size since we do not use `shareReplay` elsewhere.
Instead, we don't even really need to share the observable.

The `Observable` available in `ApplicationRef.isStable` before the above commit
was the zone stable observable, without a `share`. The new behavior adds
only an additional observable to the stream, `hasPendingTasks` (a `BehaviorSubject`).
The observables in this stream are not expensive to subscribe to. The
only one with side effects is the `isStable` (because it subscribes to
onStable), but that one already has the `share` operator on it.
Omitting the `share` in `ApplicationRef` also means that applications on `zoneless` will not
have to pay the cost of the operator when we make zones optional because
the zone stable observable is the only place we use it.
@atscott atscott added area: core Issues related to the framework runtime target: patch This PR is targeted for the next patch release labels Dec 12, 2023
@ngbot ngbot bot modified the milestone: Backlog Dec 12, 2023
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.

@atscott thanks for the fix! 👍

Copy link
Contributor

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

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

LGTM.

Initially the share was there to have a single source of truth when having multiple subscriptions to isStable.

@alxhub
Copy link
Member

alxhub commented Dec 13, 2023

This PR was merged into the repository by commit 629343f.

alxhub pushed a commit that referenced this pull request Dec 13, 2023
… see values (#53541)

The behavior of `ApplicationRef.isStable` changed in 16.1 due to
28c68f7.
This change added a `share` to the `isStable` observable, which prevents
additional subscribers from getting a value until a new one emits. One
solution to the problem would be `shareReplay(1)`. However, that would
increase the bundle size since we do not use `shareReplay` elsewhere.
Instead, we don't even really need to share the observable.

The `Observable` available in `ApplicationRef.isStable` before the above commit
was the zone stable observable, without a `share`. The new behavior adds
only an additional observable to the stream, `hasPendingTasks` (a `BehaviorSubject`).
The observables in this stream are not expensive to subscribe to. The
only one with side effects is the `isStable` (because it subscribes to
onStable), but that one already has the `share` operator on it.
Omitting the `share` in `ApplicationRef` also means that applications on `zoneless` will not
have to pay the cost of the operator when we make zones optional because
the zone stable observable is the only place we use it.

PR Close #53541
@alxhub alxhub closed this in 629343f Dec 13, 2023
atscott added a commit to atscott/angular that referenced this pull request Dec 13, 2023
… see values (angular#53541)

The behavior of `ApplicationRef.isStable` changed in 16.1 due to
angular@28c68f7.
This change added a `share` to the `isStable` observable, which prevents
additional subscribers from getting a value until a new one emits. One
solution to the problem would be `shareReplay(1)`. However, that would
increase the bundle size since we do not use `shareReplay` elsewhere.
Instead, we don't even really need to share the observable.

The `Observable` available in `ApplicationRef.isStable` before the above commit
was the zone stable observable, without a `share`. The new behavior adds
only an additional observable to the stream, `hasPendingTasks` (a `BehaviorSubject`).
The observables in this stream are not expensive to subscribe to. The
only one with side effects is the `isStable` (because it subscribes to
onStable), but that one already has the `share` operator on it.
Omitting the `share` in `ApplicationRef` also means that applications on `zoneless` will not
have to pay the cost of the operator when we make zones optional because
the zone stable observable is the only place we use it.

PR Close angular#53541
@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 Jan 13, 2024
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
… see values (angular#53541)

The behavior of `ApplicationRef.isStable` changed in 16.1 due to
angular@28c68f7.
This change added a `share` to the `isStable` observable, which prevents
additional subscribers from getting a value until a new one emits. One
solution to the problem would be `shareReplay(1)`. However, that would
increase the bundle size since we do not use `shareReplay` elsewhere.
Instead, we don't even really need to share the observable.

The `Observable` available in `ApplicationRef.isStable` before the above commit
was the zone stable observable, without a `share`. The new behavior adds
only an additional observable to the stream, `hasPendingTasks` (a `BehaviorSubject`).
The observables in this stream are not expensive to subscribe to. The
only one with side effects is the `isStable` (because it subscribes to
onStable), but that one already has the `share` operator on it.
Omitting the `share` in `ApplicationRef` also means that applications on `zoneless` will not
have to pay the cost of the operator when we make zones optional because
the zone stable observable is the only place we use it.

PR Close angular#53541
rlmestre pushed a commit to rlmestre/angular that referenced this pull request Jan 26, 2024
… see values (angular#53541)

The behavior of `ApplicationRef.isStable` changed in 16.1 due to
angular@28c68f7.
This change added a `share` to the `isStable` observable, which prevents
additional subscribers from getting a value until a new one emits. One
solution to the problem would be `shareReplay(1)`. However, that would
increase the bundle size since we do not use `shareReplay` elsewhere.
Instead, we don't even really need to share the observable.

The `Observable` available in `ApplicationRef.isStable` before the above commit
was the zone stable observable, without a `share`. The new behavior adds
only an additional observable to the stream, `hasPendingTasks` (a `BehaviorSubject`).
The observables in this stream are not expensive to subscribe to. The
only one with side effects is the `isStable` (because it subscribes to
onStable), but that one already has the `share` operator on it.
Omitting the `share` in `ApplicationRef` also means that applications on `zoneless` will not
have to pay the cost of the operator when we make zones optional because
the zone stable observable is the only place we use it.

PR Close angular#53541
danieljancar pushed a commit to danieljancar/angular that referenced this pull request Jan 26, 2024
… see values (angular#53541)

The behavior of `ApplicationRef.isStable` changed in 16.1 due to
angular@28c68f7.
This change added a `share` to the `isStable` observable, which prevents
additional subscribers from getting a value until a new one emits. One
solution to the problem would be `shareReplay(1)`. However, that would
increase the bundle size since we do not use `shareReplay` elsewhere.
Instead, we don't even really need to share the observable.

The `Observable` available in `ApplicationRef.isStable` before the above commit
was the zone stable observable, without a `share`. The new behavior adds
only an additional observable to the stream, `hasPendingTasks` (a `BehaviorSubject`).
The observables in this stream are not expensive to subscribe to. The
only one with side effects is the `isStable` (because it subscribes to
onStable), but that one already has the `share` operator on it.
Omitting the `share` in `ApplicationRef` also means that applications on `zoneless` will not
have to pay the cost of the operator when we make zones optional because
the zone stable observable is the only place we use it.

PR Close angular#53541
amilamen pushed a commit to amilamen/angular that referenced this pull request Jan 26, 2024
… see values (angular#53541)

The behavior of `ApplicationRef.isStable` changed in 16.1 due to
angular@28c68f7.
This change added a `share` to the `isStable` observable, which prevents
additional subscribers from getting a value until a new one emits. One
solution to the problem would be `shareReplay(1)`. However, that would
increase the bundle size since we do not use `shareReplay` elsewhere.
Instead, we don't even really need to share the observable.

The `Observable` available in `ApplicationRef.isStable` before the above commit
was the zone stable observable, without a `share`. The new behavior adds
only an additional observable to the stream, `hasPendingTasks` (a `BehaviorSubject`).
The observables in this stream are not expensive to subscribe to. The
only one with side effects is the `isStable` (because it subscribes to
onStable), but that one already has the `share` operator on it.
Omitting the `share` in `ApplicationRef` also means that applications on `zoneless` will not
have to pay the cost of the operator when we make zones optional because
the zone stable observable is the only place we use it.

PR Close angular#53541
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.

None yet

4 participants