Skip to content

feat(service-worker): allow specifying maxAge for entire application #49601

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.

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

Closed

Conversation

henry-alakazhang
Copy link
Contributor

@henry-alakazhang henry-alakazhang commented Mar 27, 2023

This commit adds an applicationMaxAge to the service worker configuration. When set, it will only assign a cached version to clients within the maxAge. Afterwards, it will ignored any expired application versions and fetch exclusively from the network. The default is undefined, for which the behaviour is the same as it currently is.

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: #39266

The index file (and therefore all application navigations) are cached indefinitely by the service worker.

What is the new behavior?

An optional applicationMaxAge field in the service worker ignores the cache for all requests for versions older than that age.

Does this PR introduce a breaking change?

  • Yes
  • No

(field is optional)

Other information

Sorry, something went wrong.

@pullapprove pullapprove bot requested a review from alxhub March 27, 2023 04:55
@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Mar 27, 2023
@henry-alakazhang
Copy link
Contributor Author

henry-alakazhang commented Mar 27, 2023

Wasn't sure exactly what API to use / where to put it - I've put it in the AppVersion fetch, but the logic could live the driver.ts as well.

Linked issue could also be solved by setting maxAge on asset groups, I think, but that felt a bit roundabout for dealing with navigation requests, and might also require a bit of refactoring / duplication because the logic already exists for data groups.

@henry-alakazhang
Copy link
Contributor Author

Hmm, I've realised that this doesn't work properly if the service worker process is shut down, as it will recreate the state and reset installTime. Instead this probably needs to use the manifest timestamp or persist the lastUpdateCheck to the DB

@dylhunn dylhunn added the area: service-worker Issues related to the @angular/service-worker package label Apr 4, 2023
@ngbot ngbot bot added this to the Backlog milestone Apr 4, 2023
@thePunderWoman
Copy link
Contributor

@henry-alakazhang Sorry this never got a code review. We think it's a reasonable change. Can you rebase? I'll take a look after you're able to do that.

@thePunderWoman thePunderWoman self-requested a review October 9, 2024 14:59
@thePunderWoman thePunderWoman added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Oct 9, 2024
This commit adds an `applicationMaxAge` to the service worker configuration. When set, it will only assign a cached version to clients within the `maxAge`. Afterwards, it will ignored any expired application versions and fetch exclusively from the network. The default is `undefined`, for which the behaviour is the same as it currently is.
@henry-alakazhang
Copy link
Contributor Author

henry-alakazhang commented Oct 10, 2024

Sure, I've updated the PR (hopefully I rebased everything correctly and these things are all still in the right place).

@henry-alakazhang
Copy link
Contributor Author

Hi @thePunderWoman, would you be able to take a look at this now?

Copy link
Contributor

@thePunderWoman thePunderWoman left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for your contribution!

@thePunderWoman thePunderWoman added action: review The PR is still awaiting reviews from at least one requested reviewer target: major This PR is targeted for the next major release 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 PullApprove: disable and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews action: review The PR is still awaiting reviews from at least one requested reviewer labels Oct 22, 2024
@thePunderWoman
Copy link
Contributor

Caretaker note: this is safe to merge with current approvals as it was discussed during a triage meeting.

@AndrewKushnir
Copy link
Contributor

This PR was merged into the repository by commit 8ddce80.

The changes were merged into the following branches: main

JsantosDK pushed a commit to JsantosDK/angular that referenced this pull request Nov 11, 2024
…ngular#49601)

This commit adds an `applicationMaxAge` to the service worker configuration. When set, it will only assign a cached version to clients within the `maxAge`. Afterwards, it will ignored any expired application versions and fetch exclusively from the network. The default is `undefined`, for which the behaviour is the same as it currently is.

PR Close angular#49601
@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 22, 2024
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: service-worker Issues related to the @angular/service-worker 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 PullApprove: disable target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants