GoogleChrome / GoogleChrome/workbox

Standardize behavior of `maxAgeSeconds`

Open
#3,317 0 comments 1 reaction 0 assignees View on GitHub
Dominant language
JavaScript
Stars
13k
Forks
880
Avg merge
2h 44m
Merged PRs (30d)
8

Description

Hello new team :)

I found a bug in https://github.com/GoogleChrome/workbox/issues/2863#issuecomment-881791503 and decided now that it should probably have a separate issue.

**Library Affected**:
_workbox-expiration._

**Issue or Feature Request Description**:

The documentation for what `maxAgeSeconds` does, is pretty vague:
> The maximum age of an entry before it's treated as stale and removed.

...so I did some manual testing and came to the conclusion that it must be calculating the "age" as the time since the resource was downloaded form the server. And because of that assumption, I created an entire [feature request for `maxUnusedSeconds` ](https://github.com/GoogleChrome/workbox/issues/3316) which calculates the age as the time since the resource was last used on the page.

But now it turns out, the existing implementation is actually doing a bit of both. The lightweight `Date` check you're doing in [`isResponseDateFresh`](https://github.com/GoogleChrome/workbox/blob/46af63c1780955345c117c63c8c8dd54f3d40220/packages/workbox-expiration/src/ExpirationPlugin.ts#L155) is correctly determining the time since the resource was last downloaded from the server, but for the `IndexedDB` check in [`expireEntries`](https://github.com/GoogleChrome/workbox/blob/46af63c1780955345c117c63c8c8dd54f3d40220/packages/workbox-expiration/src/ExpirationPlugin.ts#L160) to also work correctly, the timestamp must only be updated when a new version of the resource is downloaded from the server, and that is not currently the case, since you call `updateTimestamp` as part of [`cachedResponseWillBeUsed`](https://github.com/GoogleChrome/workbox/blob/46af63c1780955345c117c63c8c8dd54f3d40220/packages/workbox-expiration/src/ExpirationPlugin.ts#L145):
https://github.com/GoogleChrome/workbox/blob/46af63c1780955345c117c63c8c8dd54f3d40220/packages/workbox-expiration/src/ExpirationPlugin.ts#L162-L164

In practice, this means that the behavior of the `ExpirationPlugin` can drastically change based on subtle differences. As an example, if you save your images with the `CacheFirst` strategy and set `maxAgeSeconds` to a month, then as long as all your images are served with a correct `Date` header, they will all be removed and get a fresh update from the server at least once a month. But if you one day accidentally disable the `Date` header on your server, then the serviceworker will fall back to `IndexedDB` timestamps, which are updated every time the images are used, so if a user watches those images at least once a month for several years, then the images will just stay in cache and never be redownloaded from the server for all those years.

I believe the expected behavior is to always calculate the "age" as the time since the resource was downloaded form the server, and therefore you should make the `IndexedDB` calls adhere to that standard.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.