Skip to content

Conversation

@alan-agius4
Copy link
Contributor

@alan-agius4 alan-agius4 commented Apr 4, 2023

This commit adds support by default for HTTP caching when using provideClientHydration. Users can opt-out of this behaviour by using the withoutHttpTransferCache feature.

import {
  bootstrapApplication,
  provideClientHydration,
  withNoHttpTransferCache,
} from '@angular/platform-browser';
// ...
bootstrapApplication(RootCmp, {
  providers: [provideClientHydration(withNoHttpTransferCache())]
});

@pullapprove pullapprove bot requested a review from alxhub April 4, 2023 09:36
@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Apr 4, 2023
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are HydrationFeature and HydrationFeatureKind really needed to be part of the public API? In @angular/common/http I noticed they are but was wondering what they are needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I followed the typings that we have in the Router, but I'm ok with this option too 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should they still be exported?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, we need to export them.

@alan-agius4 alan-agius4 added this to the v16-candidates milestone Apr 4, 2023
@ngbot ngbot bot removed this from the v16-candidates milestone Apr 4, 2023
@alan-agius4 alan-agius4 added the target: major This PR is targeted for the next major release label Apr 4, 2023
@alan-agius4 alan-agius4 requested review from AndrewKushnir and removed request for alxhub April 4, 2023 09:39
@alan-agius4 alan-agius4 added action: review The PR is still awaiting reviews from at least one requested reviewer area: common Issues related to APIs in the @angular/common package labels Apr 4, 2023
@ngbot ngbot bot added this to the Backlog milestone Apr 4, 2023
@alan-agius4 alan-agius4 removed this from the Backlog milestone Apr 4, 2023
@ngbot ngbot bot added this to the Backlog milestone Apr 4, 2023
@alan-agius4 alan-agius4 modified the milestones: Backlog, v16-candidates Apr 4, 2023
@alan-agius4 alan-agius4 force-pushed the expose-http-cache branch 2 times, most recently from a6ede8b to df6bd17 Compare April 4, 2023 10:04
@alan-agius4
Copy link
Contributor Author

alan-agius4 commented Apr 4, 2023

Caretaker note: this requires a G3 patch and BUILD file to be updated. http://cl/521719612

The broken target seems to be caused by other changes in the presubmit CL.

@alan-agius4 alan-agius4 added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Apr 4, 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.

LGTM, just a quick question on the API.

Copy link
Contributor

Choose a reason for hiding this comment

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

I followed the typings that we have in the Router, but I'm ok with this option too 👍

…deClientHydration`

This commit adds support by default for HTTP caching when using `provideClientHydration`. Users can opt-out of this behaviour by using the `withoutHttpTransferCache` feature.

```ts
import {
  bootstrapApplication,
  provideClientHydration,
  withNoHttpTransferCache,
} from '@angular/platform-browser';
// ...
bootstrapApplication(RootCmp, {
  providers: [provideClientHydration(withNoHttpTransferCache())]
});
```
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.

@alan-agius4 looks great! 👍

@pullapprove pullapprove bot requested review from alxhub and atscott April 4, 2023 18:18
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.

Reviewed-for: public-api

Copy link
Contributor

@jessicajaniuk jessicajaniuk 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: fw-core, fw-http, fw-platform-server, public-api

@alan-agius4 alan-agius4 removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Apr 4, 2023
@alan-agius4 alan-agius4 added the action: merge The PR is ready for merge by the caretaker label Apr 4, 2023
@AndrewKushnir
Copy link
Contributor

Caretaker notes:

  • this PR is ready for merge
  • this change requires a G3 patch and BUILD file to be updated: http://cl/521719612 (just reposting this from the comment above for visibility)
  • the presubmit is "green" (only unrelated failures)

@dylhunn
Copy link
Contributor

dylhunn commented Apr 4, 2023

This PR was merged into the repository by commit 81e7d15.

@dylhunn dylhunn closed this in 81e7d15 Apr 4, 2023
@alan-agius4 alan-agius4 deleted the expose-http-cache branch April 5, 2023 05:03
@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 May 6, 2023
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: common Issues related to APIs in the @angular/common package 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

Development

Successfully merging this pull request may close these issues.

4 participants