Skip to content

feat(http): add keepalive support for fetch requests#60621

Closed
SkyZeroZx wants to merge 2 commits intoangular:mainfrom
SkyZeroZx:main
Closed

feat(http): add keepalive support for fetch requests#60621
SkyZeroZx wants to merge 2 commits intoangular:mainfrom
SkyZeroZx:main

Conversation

@SkyZeroZx
Copy link
Copy Markdown
Contributor

@SkyZeroZx SkyZeroZx commented Mar 29, 2025 •
edited
Loading

This commit adds support for the Fetch API's keepalive option when using HttpClient with the withFetch provider.

The change includes:

  • Added keepalive to HttpRequestInit interface
  • Modified FetchBackend to pass the option
  • Added some unit test

BREAKING CHANGE: None (pure additive feature)

Motivation/Use Cases

The keepalive option is particularly useful for:

  • Beacon API-like functionality (logging analytics on page unload)
  • Performance optimization for repeated requests to the same endpoint
  • Better control over connection management in long-lived applications

Proposed Solution

  1. Add keepalive to the HttpRequestInit interface
  2. Implement property handling in HttpRequest class
  3. Pass the option through FetchBackend to the native fetch API
  4. Maintain full backward compatibility

Examples of New Usage

// Simple GET with keepalive
http.get('/api/data', { keepalive: true }).subscribe();

// POST request with keepalive and other options
http.post('/api/log', analyticsData, {
  keepalive: true,
  headers: new HttpHeaders({'Content-Type': 'application/json'})
}).subscribe();

@pullapprove pullapprove bot requested a review from crisbeto March 29, 2025 01:04
@angular-robot angular-robot bot added detected: breaking change PR contains a commit with a breaking change detected: feature PR contains a feature commit area: common/http Issues related to HTTP and HTTP Client labels Mar 29, 2025
@ngbot ngbot bot added this to the Backlog milestone Mar 29, 2025
@SkyZeroZx SkyZeroZx mentioned this pull request Mar 29, 2025
Closed
@JeanMeche
Copy link
Copy Markdown
Member

JeanMeche commented Mar 29, 2025 •
edited
Loading

Since this is a fetch API only feature, I'd espect us to log a warning in devMode when using the XhrBackend (which is still the default backend at the time of writing this comment).

JeanMeche
JeanMeche reviewed Mar 29, 2025
View reviewed changes
@SkyZeroZx
Copy link
Copy Markdown
Contributor Author

SkyZeroZx commented Mar 30, 2025

Show a warning when using keepalive in devMode with XHR instead of the Fetch API.

JeanMeche
JeanMeche reviewed Mar 30, 2025
View reviewed changes
@JeanMeche
Copy link
Copy Markdown
Member

JeanMeche commented Apr 2, 2025 •
edited
Loading

Can you make sure to drop that merge commit.

@SkyZeroZx
Copy link
Copy Markdown
Contributor Author

SkyZeroZx commented Apr 2, 2025

Can you make sur to drop that merge commit.

Now I do a rebase to remove the merge commit

@JeanMeche
Copy link
Copy Markdown
Member

JeanMeche commented Apr 2, 2025

You'll probably have to update your main branch before rebasing to resolve that conflict.

@pullapprove pullapprove bot added the requires: TGP This PR requires a passing TGP before merging is allowed label Apr 2, 2025
@pullapprove pullapprove bot removed the requires: TGP This PR requires a passing TGP before merging is allowed label Apr 2, 2025
@JeanMeche JeanMeche added the target: major This PR is targeted for the next major release label Apr 3, 2025
@JeanMeche
Copy link
Copy Markdown
Member

JeanMeche commented Apr 3, 2025

Could you please amend the commit message and remove the breaking change mention.
Our release note generation will pick up the deprecation notice even if it's not one.

@JeanMeche JeanMeche modified the milestones: Backlog, v20 candidates Apr 3, 2025
SkyZeroZx added 2 commits April 3, 2025 16:00
@SkyZeroZx
feat(http): add keepalive support for fetch requests
0889f19 
This commit adds support for the Fetch API's keepalive option when using HttpClient with the withFetch provider.

The change includes:
- Added keepalive to HttpRequestInit interface
- Modified FetchBackend to pass the option
- Added some unit test
@SkyZeroZx
fixup! feat(http): add keepalive support for fetch requests
e49c43c
@angular-robot angular-robot bot removed the detected: breaking change PR contains a commit with a breaking change label Apr 3, 2025
@SkyZeroZx
Copy link
Copy Markdown
Contributor Author

SkyZeroZx commented Apr 3, 2025

Could you please amend the commit message and remove the breaking change mention. Our release note generation will pick up the deprecation notice even if it's not one.

It's ready, is there anything else missing?

@JeanMeche
Copy link
Copy Markdown
Member

JeanMeche commented Apr 3, 2025

We still need an additional reviewer to approve, my approval scope doesn't cover that the http module.

@SkyZeroZx
Copy link
Copy Markdown
Contributor Author

SkyZeroZx commented Apr 14, 2025

@crisbeto Is there anything missing to get approval for the merger?

@SkyZeroZx
Copy link
Copy Markdown
Contributor Author

SkyZeroZx commented Apr 17, 2025 •
edited
Loading

@JeanMeche Some tests failed even though they passed; it seems to be due to a timeout or other reason.
Is it possible to perform a rerun?

@alxhub alxhub self-assigned this Apr 24, 2025
@alxhub alxhub added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels Apr 24, 2025
@alxhub
Copy link
Copy Markdown
Member

alxhub commented Apr 24, 2025

Caretaker: this can merge, TAP is "green"

@pullapprove pullapprove bot requested review from devversion and kirjs April 24, 2025 22:02
mmalerba
mmalerba approved these changes Apr 24, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

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

reviewed-for: public-api

alxhub
alxhub approved these changes Apr 24, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@alxhub alxhub left a comment

Choose a reason for hiding this comment

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

Reviewed-for: public-api

@mmalerba
Copy link
Copy Markdown
Contributor

mmalerba commented Apr 24, 2025

This PR was merged into the repository by commit ccc5cc0.

The changes were merged into the following branches: main

@mmalerba mmalerba closed this in ccc5cc0 Apr 24, 2025
@csvn csvn mentioned this pull request Apr 30, 2025
Merged
@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot bot commented May 25, 2025

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 May 25, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@JeanMeche JeanMeche JeanMeche approved these changes

@alxhub alxhub alxhub approved these changes

+1 more reviewer

@mmalerba mmalerba mmalerba approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@alxhub alxhub

Labels

action: merge The PR is ready for merge by the caretaker area: common/http Issues related to HTTP and HTTP Client detected: feature PR contains a feature commit merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: major This PR is targeted for the next major release

Projects

None yet

Milestone

v20 candidates

Development

Successfully merging this pull request may close these issues.

4 participants

@SkyZeroZx @JeanMeche @alxhub @mmalerba