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(http): Only contribute to ApplicationRef.isStable indicator in … #55075

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

atscott
Copy link
Contributor

@atscott atscott commented Mar 27, 2024

…the HttpBackend

This commit updates the approach to how the http package contributes to application stability by moving the contribution point from an interceptor wrapper to the backend.

HttpClient uses the PendingTasks service to contribute to application stability. This was added in v16 to support SSR without relying on an infinite setTimeout with ZoneJS like it did pre-v16. Prior to version 16, this was also only done on the server and did not affect clients or unit tests. (28c68f7)

=== Additional background information ===

Prior to #54949, PendingTasks contribute to ApplicationRef.isStable but did not contribute to the stability of ComponentFixture. This divergence in stability behavior was not intended. By aligning the two behaviors again, this includes all pending tasks in the stability of fixtures. This is likely to be a pretty large breaking change test with HttpClient. Tests appear to quite often use await fixture.whenStable when there are unfinished requests that have not been mocked or flushed.

This change prevents request in HttpClient from contributing to stability through the PendingTasks in tests by not including the stability contribution in the mock backend. In this scenario, requests need to be flushed manually for them to resolve, which is problematic for existing test suites which do not flush them before await fixture.whenStable.

BREAKING CHANGE: HttpClient interceptors are no longer contribute directly to ApplicationRef.isStable without ZoneJS. ApplicationRef.isStable is used for SSR to determine when to serialize the application. If there is async work in interceptors that is not captured by ZoneJS and needs to be included in the SSR serialization, these should be updated to keep the zone unstable by running a timeout inside the zone and clearing it when the async interceptor work is finished.

@angular-robot angular-robot bot added the detected: breaking change PR contains a commit with a breaking change label Mar 27, 2024
@ngbot ngbot bot added this to the Backlog milestone Mar 27, 2024
@atscott atscott added the target: major This PR is targeted for the next major release label Mar 27, 2024
@atscott atscott force-pushed the httpstabilityinbackend branch 2 times, most recently from 8532519 to b2193bb Compare April 1, 2024 18:33
…the `HttpBackend`

This commit updates the approach to how the `http` package contributes
to application stability by moving the contribution point from an
interceptor wrapper to the backend.

`HttpClient` uses the `PendingTasks` service to contribute to
application stability. This was added in v16 to support SSR without
relying on an infinite `setTimeout` with ZoneJS like it did pre-v16.
Prior to version 16, this was also only done on the server and did not
affect clients or unit tests. (angular@28c68f7)

=== Additional background information ===

Prior to angular#54949, `PendingTasks` contribute to `ApplicationRef.isStable` but did not
contribute to the stability of `ComponentFixture`. This divergence in
stability behavior was not intended. By aligning the two behaviors again, this
includes all pending tasks in the stability of fixtures. This is
likely to be a pretty large breaking change test with `HttpClient`.
Tests appear to quite often use `await fixture.whenStable` when there are
unfinished requests that have not been mocked or flushed.

This change prevents request in `HttpClient` from contributing to
stability through the `PendingTasks` in tests by not including the stability
contribution in the mock backend. In this scenario, requests need to be
flushed manually for them to resolve, which is problematic for existing
test suites which do not flush them before `await fixture.whenStable`.

BREAKING CHANGE: `HttpClient` interceptors are no longer contribute directly to
`ApplicationRef.isStable` without ZoneJS. `ApplicationRef.isStable` is
used for SSR to determine when to serialize the application. If there is
async work in interceptors that is not captured by ZoneJS and needs to
be included in the SSR serialization, these should be updated to keep
the zone unstable by running a timeout inside the zone and clearing it when
the async interceptor work is finished.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: common/http detected: breaking change PR contains a commit with a breaking change target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants