fix(http): force macro task creation during request #49546
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
8bd7c3b to
0814906
Compare
0814906 to
9fa49a0
Compare
9fa49a0 to
d83f94d
Compare
Contributor
Author
|
//cc @AndrewKushnir, not sure if you want to review this, as we discussed about this a bit yesterday. |
alan-agius4
commented
Mar 23, 2023
3 tasks
4f02e42 to
d83f94d
Compare
AndrewKushnir
approved these changes
Mar 27, 2023
Contributor
AndrewKushnir
left a comment
There was a problem hiding this comment.
@alan-agius4 looks great 👍
We'd probably need a TGP for this change.
packages/common/http/src/xhr.ts
Outdated
Contributor
There was a problem hiding this comment.
Can we use Number.MAX_VALUE here? If not, could you plz add a comment into the code, so we have this context in the future.
Contributor
Author
There was a problem hiding this comment.
Added comment why.
Contributor
There was a problem hiding this comment.
What about Number.MAX_SAFE_INTEGER? That's basically what it is for, right?
Contributor
Author
There was a problem hiding this comment.
No, Number.MAX_SAFE_INTEGER is a 53 bit integer.
packages/platform-server/src/http.ts
Outdated
Contributor
There was a problem hiding this comment.
Love the removal of this code (in favor of a more simple solution!) 👍
jessicajaniuk
approved these changes
Mar 27, 2023
Contributor
jessicajaniuk
left a comment
There was a problem hiding this comment.
reviewed-for: fw-http, fw-platform-server, integration-tests, public-api
This commit adds a background macrotask when an XHR request is performed. The macrotask is started during `loadstart` and ended during `loadend` event. The macrotask is needed so that the application is not stabilized during HTTP calls. This is important for server rendering, as the application is rendering when the application is stabilized. The application is stabilized when there are no longer pending Macro and Micro tasks intercepted by Zone.js, Since an XHR request is none of these, we create a background macrotask so that Zone.js is made aware that there is something pending. Prior to this change, we patched the `HttpHandler` in `@angular/platform-server` but this is not enough, as there can be multiple `HttpHandler` in an application, example when importing `HttpClient` in a lazy loaded component/module. Which causes a new unpatched instance of `HttpHandler` to be created in the child injector which is not intercepted by Zone.js and thus the application is stabalized and rendered before the XHR request is finalized. NB: Zone.js is fundamental for SSR and currently, it's not possible to do SSR without it. Closes: angular#49425
These two options where created for a feature which was never completed. angular/universal#1860 Eventually these options should be added to `withTransferCache` HTTP logic. DEPRECATED: `PlatformConfig.baseUrl` and `PlatformConfig.useAbsoluteUrl` platform-server config options are deprecated as these were not used.
c5e2257 to
b896849
Compare
Contributor
Author
AndrewKushnir
approved these changes
Mar 28, 2023
Contributor
AndrewKushnir
left a comment
There was a problem hiding this comment.
Reviewed-for: public-api
Contributor
|
This PR was merged into the repository by commit e994608. |
atscott
pushed a commit
that referenced
this pull request
Mar 29, 2023
…49546) These two options where created for a feature which was never completed. angular/universal#1860 Eventually these options should be added to `withTransferCache` HTTP logic. DEPRECATED: `PlatformConfig.baseUrl` and `PlatformConfig.useAbsoluteUrl` platform-server config options are deprecated as these were not used. PR Close #49546
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
refactor(platform-server): deprecate
useAbsoluteUrlandbaseUrlThese two options where created for a feature which was never completed. angular/universal#1860
Eventually these options should be added to
withTransferCacheHTTP logic.DEPRECATED:
PlatformConfig.baseUrlandPlatformConfig.useAbsoluteUrlplatform-server config options are deprecated as these were not used.fix(http): force macro task creation during HTTP request
This commit adds a background macrotask when an XHR request is performed. The macrotask is started during
loadstartand ended duringloadendevent.The macrotask is needed so that the application is not stabilized during HTTP calls. This is important for server rendering, as the application is rendering when the application is stabilized.
The application is stabilized when there are no longer pending Macro and Micro tasks intercepted by Zone.js, Since an XHR request is none of these, we create a background macrotask so that Zone.js is
made aware that there is something pending.
Prior to this change, we patched the
HttpHandlerin@angular/platform-serverbut this is not enough, as there can be multipleHttpHandlerin an application, example when importingHttpClientin a lazy loaded component/module.Which causes a new unpatched instance of
HttpHandlerto be created in the child injector which is not intercepted by Zone.js and thus the application is stabalized and rendered before the XHR request is finalized.NB: Zone.js is fundamental for SSR and currently, it's not possible to do SSR without it.
Closes: #49425