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.

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

Support sub request cache control header stale-while-revalidate everywhere #1049

Merged
merged 12 commits into from
Apr 7, 2022

Conversation

wizardlyhel
Copy link
Collaborator

@wizardlyhel wizardlyhel commented Apr 6, 2022

Fix: https://github.com/Shopify/custom-storefronts/issues/175

Description

This is a workaround for supporting stale-while-revalidate cache control on Cloudflare everywhere

How to test on Cloudflare

You must have:

  • A Cloudflare domain (cannot be the worker.dev that Cloudflare gives you)
  • A worker route that point to the Cloudflare domain

Once you have satisfy the above requirements, you can deploy your app with setLoggerOptions({showCacheApiStatus: true}) and wrangler tail -f pretty to see the cache api logs.

Before submitting the PR, please make sure you do the following:

  • Read the Contributing Guidelines
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123)
  • Update docs in this repository according to your change
  • Run yarn changeset add if this PR cause a version bump based on Keep a Changelog and adheres to Semantic Versioning

@github-actions

This comment has been minimized.

Copy link
Contributor

@jplhomer jplhomer left a comment

Choose a reason for hiding this comment

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

Super cool fix! As mentioned in the comments, I think we should remove forked logic for Cloudflare and instead behave this way for all runtimes.

packages/hydrogen/src/framework/cache.ts Outdated Show resolved Hide resolved
packages/hydrogen/src/framework/cache.ts Outdated Show resolved Hide resolved
packages/hydrogen/src/framework/cache.ts Outdated Show resolved Hide resolved
@wizardlyhel wizardlyhel changed the title Fix Cloudflare cache API integration Support sub request cache control header stale-while-revalidate everywhere Apr 6, 2022
Copy link
Contributor

@jplhomer jplhomer left a comment

Choose a reason for hiding this comment

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

🚀 Amazing work!

@@ -17,7 +17,7 @@ export class InMemoryCache {
}

put(request: Request, response: Response) {
logCacheApiStatus('PUT', request.url);
logCacheApiStatus('PUT-dev', request.url);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we'll be using this in production soon Shopify/hydrogen#977 do we need the -dev suffixes? Not a blocker — we can address in a future PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The reason I put the -dev suffixes is because this in-memory cache can only be turned on if devCache is turn on. I want it to be obvious that it is a cache for development purpose and not for production purpose.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let's leave it for now and address in that future PR

@wizardlyhel wizardlyhel merged commit b88a885 into v1.x-2022-07 Apr 7, 2022
@wizardlyhel wizardlyhel deleted the cf-cache-revalidate branch April 7, 2022 16:51
blittle added a commit that referenced this pull request Apr 11, 2022
* v1.x-2022-07:
  [Hydrogen docs]: Analytics (#962)
  Analytics (#890)
  Allow line breaks inside script tags (#1061)
  Fix encoding of CSS Modules to solve hydration errors (#1062)
  [Hydrogen docs]: Update guidance on troubleshooting third-party dependencies (#1055)
  Update Vite RSC Plugin (#1054)
  Fix link prefetch (#1059)
  Initial devTools component (#998)
  Support sub request cache control header `stale-while-revalidate` everywhere (#1049)
  Fix cookie parsing (#1046)
  Update 3-external-images-with-a-custom-loader.example.tsx (#1045)
  Restore scroll position on browser navigation (#1047)
  Remove old changeset to unblock release (#1042)
  [ci] release v1.x-2022-07 (#1019)
  Update to Vite 2.9 (#1039)
  Use lang directive (#1028)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants