fix(http): force macro task creation during request #49546

alan-agius4 · 2023-03-22T15:15:35Z

refactor(platform-server): deprecate useAbsoluteUrl and baseUrl

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.

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 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: #49425

alan-agius4 · 2023-03-23T12:11:22Z

//cc @AndrewKushnir, not sure if you want to review this, as we discussed about this a bit yesterday.

packages/common/http/src/xhr.ts

AndrewKushnir

@alan-agius4 looks great 👍

We'd probably need a TGP for this change.

packages/common/http/src/xhr.ts

AndrewKushnir · 2023-03-27T22:26:18Z

packages/common/http/src/xhr.ts

@@ -320,3 +337,20 @@ export class HttpXhrBackend implements HttpBackend {
    });
  }
 }
+
+const MAX_INT = 2147483647;


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.

Added comment why.

What about Number.MAX_SAFE_INTEGER? That's basically what it is for, right?

No, Number.MAX_SAFE_INTEGER is a 53 bit integer.

AndrewKushnir · 2023-03-27T22:27:50Z

packages/platform-server/src/http.ts

 @Injectable()
 export class ServerXhr implements XhrFactory {
  build(): XMLHttpRequest {
    return new xhr2.XMLHttpRequest();
  }
 }

-export abstract class ZoneMacroTaskWrapper<S, R> {


Love the removal of this code (in favor of a more simple solution!) 👍

packages/platform-server/src/tokens.ts

jessicajaniuk

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.

…eUrl`

alan-agius4 · 2023-03-28T10:12:39Z

Green TGP http://test/OCL:518825107:BASE:519969012:1679996315169:97a90298

AndrewKushnir

Reviewed-for: public-api

atscott · 2023-03-29T16:13:43Z

This PR was merged into the repository by commit e994608.

…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

angular-automatic-lock-bot · 2023-04-29T00:07:57Z

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.}

alan-agius4 force-pushed the http-fix-universal branch 2 times, most recently from 8bd7c3b to 0814906 Compare March 23, 2023 10:13

angular-robot bot added the detected: deprecation PR contains a commit with a deprecation label Mar 23, 2023

alan-agius4 force-pushed the http-fix-universal branch from 0814906 to 9fa49a0 Compare March 23, 2023 10:16

alan-agius4 added area: server Issues related to server-side rendering area: common/http target: major This PR is targeted for the next major release labels Mar 23, 2023

ngbot bot modified the milestone: Backlog Mar 23, 2023

alan-agius4 force-pushed the http-fix-universal branch from 9fa49a0 to d83f94d Compare March 23, 2023 11:37

alan-agius4 marked this pull request as ready for review March 23, 2023 12:10

alan-agius4 added the action: review The PR is still awaiting reviews from at least one requested reviewer label Mar 23, 2023

pullapprove bot requested review from atscott and jessicajaniuk March 23, 2023 12:10

alan-agius4 commented Mar 23, 2023

View reviewed changes

packages/common/http/src/xhr.ts Outdated Show resolved Hide resolved

AndrewKushnir mentioned this pull request Mar 24, 2023

refactor(core): internal tracker of pending tasks during initial rendering #49576

Closed

3 tasks

alan-agius4 force-pushed the http-fix-universal branch 2 times, most recently from 4f02e42 to d83f94d Compare March 24, 2023 13:48

alan-agius4 requested a review from AndrewKushnir March 27, 2023 17:38

AndrewKushnir approved these changes Mar 27, 2023

View reviewed changes

pullapprove bot requested a review from dylhunn March 27, 2023 22:32

AndrewKushnir added the action: global presubmit The PR is in need of a google3 global presubmit label Mar 27, 2023

jessicajaniuk approved these changes Mar 27, 2023

View reviewed changes

pullapprove bot requested a review from pkozlowski-opensource March 27, 2023 23:03

alan-agius4 added 4 commits March 28, 2023 07:23

fixup! fix(http): force macro task creation during HTTP request

f7bbefa

fixup! refactor(platform-server): deprecate useAbsoluteUrl and `bas…

b896849

…eUrl`

alan-agius4 force-pushed the http-fix-universal branch from c5e2257 to b896849 Compare March 28, 2023 07:23

fixup! fix(http): force macro task creation during HTTP request

3566cc4

alan-agius4 added merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed action: global presubmit The PR is in need of a google3 global presubmit labels Mar 28, 2023

AndrewKushnir approved these changes Mar 28, 2023

View reviewed changes

alan-agius4 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 Mar 28, 2023

alan-agius4 removed request for atscott, pkozlowski-opensource and dylhunn March 28, 2023 18:50

alan-agius4 removed the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Mar 29, 2023

atscott closed this in 45a6ac0 Mar 29, 2023

alan-agius4 deleted the http-fix-universal branch March 29, 2023 17:25

angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(http): force macro task creation during request #49546

fix(http): force macro task creation during request #49546

alan-agius4 commented Mar 22, 2023 •

edited

alan-agius4 commented Mar 23, 2023

AndrewKushnir left a comment

AndrewKushnir Mar 27, 2023

alan-agius4 Mar 28, 2023 •

edited

alfaproject Mar 28, 2023

alan-agius4 Mar 28, 2023 •

edited

AndrewKushnir Mar 27, 2023

jessicajaniuk left a comment

alan-agius4 commented Mar 28, 2023

AndrewKushnir left a comment

atscott commented Mar 29, 2023

angular-automatic-lock-bot bot commented Apr 29, 2023

fix(http): force macro task creation during request #49546

fix(http): force macro task creation during request #49546

Conversation

alan-agius4 commented Mar 22, 2023 • edited

alan-agius4 commented Mar 23, 2023

AndrewKushnir left a comment

Choose a reason for hiding this comment

AndrewKushnir Mar 27, 2023

Choose a reason for hiding this comment

alan-agius4 Mar 28, 2023 • edited

Choose a reason for hiding this comment

alfaproject Mar 28, 2023

Choose a reason for hiding this comment

alan-agius4 Mar 28, 2023 • edited

Choose a reason for hiding this comment

AndrewKushnir Mar 27, 2023

Choose a reason for hiding this comment

jessicajaniuk left a comment

Choose a reason for hiding this comment

alan-agius4 commented Mar 28, 2023

AndrewKushnir left a comment

Choose a reason for hiding this comment

atscott commented Mar 29, 2023

angular-automatic-lock-bot bot commented Apr 29, 2023

alan-agius4 commented Mar 22, 2023 •

edited

alan-agius4 Mar 28, 2023 •

edited

alan-agius4 Mar 28, 2023 •

edited