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

server-adapter: prevent to replace correct swr header #162

Merged
merged 5 commits into from
Sep 5, 2023

Conversation

Tietew
Copy link
Contributor

@Tietew Tietew commented Jul 19, 2023

server-adapter replaces stale-while-revalidate in Cache-Control header.
ref. https://github.com/serverless-stack/open-next#workaround-nextserver-does-not-set-correct-swr-cache-headers

When getServerSideProps sets correct swr, for example:

export const getServerSideProps = async ({ res }) => {
  // fetch my data
  res.setHeader('Cache-Control', 'max-age=60, stale-while-revalidate=86400');
  return { props: /* my data */ }
};

server-adapter generates incorrect Cache-Control:

Cache-Control: max-age=60, stale-while-revalidate=2592000=86400

This PR fixes to prevent replacing stale-while-revalidate=<duration>.

Test:

> 'max-age=60, stale-while-revalidate'.replace(/\bstale-while-revalidate(?!=)/, "stale-while-revalidate=2592000")
'max-age=60, stale-while-revalidate=2592000'
> 'max-age=60, stale-while-revalidate=86400'.replace(/\bstale-while-revalidate(?!=)/, "stale-while-revalidate=2592000")
'max-age=60, stale-while-revalidate=86400'
> 'max-age=60, stale-while-revalidate, must-revalidate'.replace(/\bstale-while-revalidate(?!=)/, "stale-while-revalidate=2592000")
'max-age=60, stale-while-revalidate=2592000, must-revalidate'
> 'max-age=60, stale-while-revalidate=86400, must-revalidate'.replace(/\bstale-while-revalidate(?!=)/, "stale-while-revalidate=2592000")
'max-age=60, stale-while-revalidate=86400, must-revalidate'

@changeset-bot
Copy link

changeset-bot bot commented Jul 19, 2023

⚠️ No Changeset found

Latest commit: cc5a100

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@seed-deploy seed-deploy bot temporarily deployed to pr162 July 19, 2023 13:01 Inactive
@vercel
Copy link

vercel bot commented Jul 19, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
open-next ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 5, 2023 4:47pm

@jayair
Copy link
Contributor

jayair commented Jul 22, 2023

Just checking if you posted about this on #open-next on Discord?

Copy link
Collaborator

@khuezy khuezy 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!

@khuezy
Copy link
Collaborator

khuezy commented Aug 30, 2023

Sorry for the long delay. I'll try to merge this by the end of week, there's some critical infra PRs that need to go in ahead of this.
Don't worry about the conflict, I'll address it along w/ some unit tests.

@khuezy
Copy link
Collaborator

khuezy commented Sep 1, 2023

Added some unit tests but haven't set up the GH actions yet, waiting for #208 first

@khuezy
Copy link
Collaborator

khuezy commented Sep 5, 2023

Hmm, sorry for the random pushes, GH was glitching... it's behaving correctly now

@khuezy khuezy merged commit 2301413 into sst:main Sep 5, 2023
3 checks passed
khuezy added a commit that referenced this pull request Sep 5, 2023
khuezy added a commit that referenced this pull request Sep 5, 2023
@khuezy
Copy link
Collaborator

khuezy commented Sep 5, 2023

Hi @Tietew I merged this PR but had to revert b/c my commits on top of it was causing the app to add quotes in the URL... not sure what was causing that, it might have been from a strange rebase... anyways your commit is in the git history so thanks for your contribution. I'll cherrypick the code in this PR: #214

Edit: the code was fine, AWS deployment was buggy. I took down the app and redeploy and it's fine... very strange.

@Tietew Tietew deleted the swr-fix branch September 6, 2023 02:30
@Tietew
Copy link
Contributor Author

Tietew commented Sep 6, 2023

@khuezy Thanks much for your work!

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.

None yet

3 participants