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.

Sign up for GitHub

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

Jump to bottom

feat(http): allow customization of the HttpTransferCache. #52029

Closed
wants to merge 1 commit into from
Closed

feat(http): allow customization of the HttpTransferCache. #52029

wants to merge 1 commit into from

Conversation

JeanMeche
Copy link
Member

@JeanMeche JeanMeche commented Oct 4, 2023 •
edited

PR previously at #51872

provideClientHydration() accepts new HydrationFeature : HttpTransferCacheOptions.

withHttpTransferCacheOptions accepts an option object:

  • includeHeaders : list of headers entries to keep in the cache with the request
  • filter a callback to determine if a request should be cached
  • includePostRequests: to include POST requests in the allowed methods

Implements some of the features requested in #50117

@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Oct 4, 2023
AndrewKushnir
AndrewKushnir reviewed Oct 4, 2023
View reviewed changes
packages/platform-browser/src/hydration.ts Outdated Show resolved Hide resolved
packages/platform-browser/src/hydration.ts Outdated Show resolved Hide resolved
packages/core/src/util/stringify.ts Outdated Show resolved Hide resolved
@alan-agius4
Copy link
Contributor

NIT, can you update the commit to reference #50117?

@JeanMeche JeanMeche force-pushed the feat/transfer-cache branch 2 times, most recently from 74caec8 to 96e731e Compare October 4, 2023 21:20
@AndrewKushnir AndrewKushnir added this to the v17-candidates milestone Oct 5, 2023
@ngbot ngbot bot removed this from the v17-candidates milestone Oct 5, 2023
AndrewKushnir
AndrewKushnir reviewed Oct 5, 2023
View reviewed changes
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.

@JeanMeche the change looks good 👍

Could you please update golden files to reflect the API changes?

packages/platform-browser/src/hydration.ts Outdated Show resolved Hide resolved
@JeanMeche JeanMeche marked this pull request as ready for review October 5, 2023 17:45
@JeanMeche JeanMeche force-pushed the feat/transfer-cache branch 2 times, most recently from ebed59f to 7b54c7d Compare October 5, 2023 17:53
@AndrewKushnir AndrewKushnir added action: review The PR is still awaiting reviews from at least one requested reviewer area: common/http target: major This PR is targeted for the next major release labels Oct 5, 2023
@ngbot ngbot bot modified the milestone: Backlog Oct 5, 2023
AndrewKushnir
AndrewKushnir approved these changes Oct 5, 2023
View reviewed changes
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

@AndrewKushnir AndrewKushnir added the action: presubmit The PR is in need of a google3 presubmit label Oct 5, 2023
@AndrewKushnir
Copy link
Contributor

Presubmit.

@AndrewKushnir
Copy link
Contributor

FYI, I've also pushed a rebase, since the setup CI job was failing (due to the base commit being too old).

jessicajaniuk
jessicajaniuk approved these changes Oct 5, 2023
View reviewed changes
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: public-api

@AndrewKushnir AndrewKushnir removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Oct 5, 2023
@AndrewKushnir AndrewKushnir removed the request for review from dylhunn October 5, 2023 22:11
@AndrewKushnir AndrewKushnir 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 and removed action: presubmit The PR is in need of a google3 presubmit labels Oct 5, 2023
@AndrewKushnir AndrewKushnir modified the milestones: Backlog, v17-candidates Oct 5, 2023
@AndrewKushnir AndrewKushnir added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed 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 Oct 6, 2023
@AndrewKushnir
Copy link
Contributor

@JeanMeche it looks like there are some merge conflicts (likely after merging other PRs into the main branch). Could you please rebase when you get a chance?

@JeanMeche
feat(http): allow customization of the HttpTransferCache.
2f17947 
`provideClientHydration()` accepts new `HydrationFeature` : `HttpTransferCacheOptions`.

`withHttpTransferCacheOptions` accepts an option object:
* `includeHeaders` : list of headers entries to keep in the cache with the request
* `filter` a callback to determine if a request should be cached
* `includePostRequests`: to include POST requests in the allowed methods

Implements some of the features requested in angular#50117
@AndrewKushnir AndrewKushnir 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 and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Oct 6, 2023
@AndrewKushnir
Copy link
Contributor

Caretaker note: presubmit is "green", this PR is ready for merge.

@atscott
Copy link
Contributor

atscott commented Oct 6, 2023

This PR was merged into the repository by commit 7dde42a.

@atscott atscott closed this in 7dde42a Oct 6, 2023
@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 Nov 6, 2023
@JeanMeche JeanMeche deleted the feat/transfer-cache branch December 24, 2023 14:15
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
@JeanMeche @ChellappanRajan
feat(http): allow customization of the HttpTransferCache. (angular#52029
97eb202 
)

`provideClientHydration()` accepts new `HydrationFeature` : `HttpTransferCacheOptions`.

`withHttpTransferCacheOptions` accepts an option object:
* `includeHeaders` : list of headers entries to keep in the cache with the request
* `filter` a callback to determine if a request should be cached
* `includePostRequests`: to include POST requests in the allowed methods

Implements some of the features requested in angular#50117

PR Close angular#52029
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@AndrewKushnir AndrewKushnir AndrewKushnir approved these changes

@jessicajaniuk jessicajaniuk jessicajaniuk approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker area: common/http 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
v17-candidates
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@JeanMeche @alan-agius4 @AndrewKushnir @atscott @jessicajaniuk