-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
fix(vercel): static headers #14039
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
base: main
Are you sure you want to change the base?
fix(vercel): static headers #14039
Conversation
🦋 Changeset detectedLatest commit: b1d6ad5 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This doesn't seem to work, unless I missed something. Deployed URL of the minimal example (patched with these changes): https://github-2y2x5yu5-kre74eab7-justin-halls-projects-df945a1f.vercel.app
{
"version": 3,
"routes": [
{
"handle": "filesystem"
},
{
"src": "^/_astro/(.*)$",
"headers": {
"cache-control": "public, max-age=31536000, immutable"
},
"continue": true
},
{
"src": "/",
"headers": {
"content-security-policy": "script-src 'self' 'sha256-BF0290pkb3jxQsE7z00xR8Imp8X34FLC88L0lkMnrGw=' 'sha256-QzWFZi+FLIx23tnm9SBU4aEgx4x8DsuASP07mfqol/c=' 'sha256-0chmwFk0zaA528yFfGV7J9ppIpdfTPPULncDF3WG7Zs=' 'sha256-eIXWvAmxkr251LJZkjniEK5LcPF3NkapbJepohwYRIc=' 'sha256-Q2BPg90ZMplYY+FSdApNErhpWafg2hcRRbndmvxuL/Q='; style-src 'self' ;"
}
}
]
} Results: |
Any thoughts on this, @ematipico? |
Not really, to be honest :/ |
@ematipico After hours of debugging, it seems to be a route ordering issue related to be the I hope this is helpful - let me know what you think. |
Thank you, @wKovacs64, for your insights. Unfortunately, I don't know what you're talking about, and I don't know if I need to fix something or if it's something related to the docs. Do you have some link where I can read more about |
I added a warning in case that field is in the first place |
@ematipico I think it must be something generated by |
This is the generated {
"version": 3,
"routes": [
{
"handle": "filesystem"
},
{
"src": "^/_astro/(.*)$",
"headers": {
"cache-control": "public, max-age=31536000, immutable"
},
"continue": true
}
]
} |
Ah, thank you @ascorbic! I opened a feature request here vercel/vercel#13631 In the meantime, I think I will patch the behaviour, and shift the entry to the last place. |
6bf1462
to
b1d6ad5
Compare
commit: |
Does this change mean that on-demand dynamic routes will match before static files? I don't really know the config format, but the implication is that it looks at the filesystem before looking at server routes. Maybe you should be adding the headers to that entry, instead of moving it? |
I didn't understand this part 🤔 |
That's certainly the implication. Just to be clear, you want the priority to be: redirects | header routes (like CSP) > |
I think he's proposing adding the headers to the |
@ematipico Anything holding this one back from getting merged? Really looking forward to this feature! |
Yes. We're unsure about the correct approach. Unfortunately the lack of documentation is holding us back from creating the correct solution |
I think we can figure it out, we just need consensus on the desired priority. Does Astro want dynamic routes or filesystem routes to match first? |
@wKovacs64, technically, yes, static routes should be prioritised. I don't know how the configuration will look with static headers for static pages. |
Changes
Closes #13996
The information regarding the headers were saved in the wrong place. This PR fixes that by putting the information in the
routes
field: https://github.com/vercel/examples/blob/main/build-output-api/routes/.vercel/output/config.jsonTesting
Updated the tests
Docs
N/A