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

Increase default cache SWR #1336

Merged
merged 9 commits into from
Oct 25, 2023
Merged

Increase default cache SWR #1336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f525432
Adds a caching option for max-age 1second, swr 1 day
benjaminsehl Sep 13, 2023
439ff26
Add changeset
benjaminsehl Sep 13, 2023
3cba653
Make CacheDefault private
benjaminsehl Sep 13, 2023
516d829
Fix broken import
benjaminsehl Sep 13, 2023
0aaceb6
Merge branch 'main' into increase-default-cache-swr
benjaminsehl Sep 20, 2023
6821651
Update four-toes-shout.md
benjaminsehl Sep 20, 2023
d086a1f
Update calendar version
wizardlyhel Oct 17, 2023
5206562
Merge remote-tracking branch 'origin/main' into 2023-10
wizardlyhel Oct 20, 2023
b461a0c
Merge remote-tracking branch 'origin/2023-10' into increase-default-c…
wizardlyhel Oct 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/four-toes-shout.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/hydrogen': patch
Copy link
Contributor

@abecciu abecciu Sep 14, 2023 •
edited

Choose a reason for hiding this comment

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

Shouldn't this be at least a minor release given that we are changing an important default behavior?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is the weird thing about changesets & calver … right @blittle? "Minor" would mean it gets bumped to 2023.08

This is definitely a DevOps oddity we need to resolve.

Copy link
Contributor

Choose a reason for hiding this comment

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

Using calver, following storefront api release, is a decision we agree on - So everything is a patch. If it is a breaking change, we should release as in opted-in feature or unstable feature until the next calendar release.

---

Introduce a new default caching strategy with a `max-age` value of 1 second, and a `stale-while-revalidate` value of 1 day. When updating to this version of Hydrogen, note that if you would like to continue to use `CacheShort` as the default (10 second cache) to decrease risk of stale data (but also increase load times on low traffic pages), it's important to go and manually specify your caching strategy.
2 changes: 1 addition & 1 deletion .github/workflows/changesets.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ jobs:
id: flags
run: |
# IMPORTANT: Update this latestBranch whenever we move to a new major version:
echo "latestBranch=2023-07" >> $GITHUB_ENV
echo "latestBranch=2023-10" >> $GITHUB_ENV
echo "latest=${{ github.ref_name == 'main' }}" >> $GITHUB_ENV

- name: Checkout the code
Expand Down
4 changes: 2 additions & 2 deletions packages/hydrogen/src/cache/api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type {CachingStrategy} from './strategies';
import {CacheShort, generateCacheControlHeader} from './strategies';
import {CacheDefault, generateCacheControlHeader} from './strategies';

function logCacheApiStatus(
status: string | null,
Expand Down Expand Up @@ -35,7 +35,7 @@ function getCacheControlSetting(
...options,
};
} else {
return userCacheOptions || CacheShort();
return userCacheOptions || CacheDefault();
}
}

Expand Down
16 changes: 16 additions & 0 deletions packages/hydrogen/src/cache/strategies.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,22 @@ export function CacheLong(overrideOptions?: CachingStrategy): AllCacheOptions {
};
}

/**
*
* @private
*/
export function CacheDefault(
overrideOptions?: CachingStrategy,
): AllCacheOptions {
guardExpirableModeType(overrideOptions);
return {
mode: PUBLIC,
maxAge: 1,
staleWhileRevalidate: 86399, // 1 second less than 24 hours
...overrideOptions,
};
}

/**
*
* @public
Expand Down
4 changes: 2 additions & 2 deletions packages/hydrogen/src/cache/sub-request.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {parseJSON} from '../utils/parse-json';
import {CacheAPI} from './api';
import {
CacheShort,
CacheDefault,
type CachingStrategy,
type AllCacheOptions,
} from './strategies.js';
Expand All @@ -18,7 +18,7 @@ export function getKeyUrl(key: string) {
}

function getCacheOption(userCacheOptions?: CachingStrategy): AllCacheOptions {
return userCacheOptions || CacheShort();
return userCacheOptions || CacheDefault();
}

export function generateSubRequestCacheControlHeader(
Expand Down
3 changes: 2 additions & 1 deletion packages/hydrogen/src/storefront.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
CacheNone,
CacheLong,
CacheShort,
CacheDefault,
CacheCustom,
generateCacheControlHeader,
type CachingStrategy,
Expand Down Expand Up @@ -344,7 +345,7 @@ export function createStorefrontClient<TI18n extends I18nBase>(

const [body, response] = await fetchWithServerCache(url, requestInit, {
cacheInstance: mutation ? undefined : cache,
cache: cacheOptions || CacheShort(),
cache: cacheOptions || CacheDefault(),
cacheKey,
shouldCacheResponse: checkGraphQLErrors,
waitUntil,
Expand Down