perf(netlify, netlify-edge): exclude static paths from server handler #2822
+114
−6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
serhalp
commented
Oct 30, 2024
serhalp
commented
Oct 30, 2024
serhalp
commented
Oct 30, 2024
The netlify preset was already using `preferStatic: true` which lets existing static assets take precedence over the function, but this now also avoids a function invocation for static paths that don't exist, i.e. this avoids an unnecessary dynamic 404 that could be served directly from the CDN. The `netlify-edge` preset wasn't excluding anything, so this addresses both the 404 case and the existing asset case. The 404 case is important because browsers frequently attempt to request hashed assets from previous deploys that have been invalidated. There's no reason for this to go through functions, since we know that the whole parent path is static.
serhalp
force-pushed
the
perf/netlify/avoid-static-all-asset-invocations
branch
from
5369c48
to
ce72ea3
Compare
serhalp
commented
Oct 30, 2024
pi0
reviewed
Oct 30, 2024
pi0
reviewed
Oct 30, 2024
|
Thanks for the PR dear @serhalp It seems main blockers on PR are addition of tests (which are so nice) -- although i don't want the main improvement to be missed for 2.10 release, perhaps we can extract work on tests to an additional PR? Main addition seems could be tested easily both manually or with a simple unit test for |
serhalp
commented
Oct 31, 2024
serhalp
added a commit
to serhalp/nitro
that referenced
this pull request
Oct 31, 2024
Backporting unjs#2822 to the netlify-legacy preset. Since it's using "v1" Netlify Functions (https://docs.netlify.com/functions/lambda-compatibility), it doesn't have access to `excludedPath` and `preferStatic`, so we implement this with redirects.
pi0
reviewed
Oct 31, 2024
serhalp
added a commit
to serhalp/nitro
that referenced
this pull request
Oct 31, 2024
Backporting nitrojs#2822 to the netlify-legacy preset. Since it's using "v1" Netlify Functions (https://docs.netlify.com/functions/lambda-compatibility), it doesn't have access to `excludedPath` and `preferStatic`, so we implement this with redirects.
serhalp
added a commit
to serhalp/nitro
that referenced
this pull request
Oct 31, 2024
Backporting unjs#2822 to the netlify-legacy preset. Since it's using "v1" Netlify Functions (https://docs.netlify.com/functions/lambda-compatibility), it doesn't have access to `excludedPath` and `preferStatic`, so we implement this with redirects.
8 tasks
pi0
changed the title
serhalp
added a commit
to serhalp/nitro
that referenced
this pull request
Oct 31, 2024
Backporting unjs#2822 to the netlify-legacy preset. Since it's using "v1" Netlify Functions (https://docs.netlify.com/functions/lambda-compatibility), it doesn't have access to `excludedPath` and `preferStatic`, so we implement this with redirects.
and unit test it also, ensure the right URL format on paths
|
@pi0 I made all the requested changes. Here's a manual QA against a nuxt site (opted in to nuxt v4 + 2014-10-31 compat date for new preset):
|
|
And for good measure here's one against
|
serhalp
commented
Oct 31, 2024
pi0
approved these changes
Oct 31, 2024
|
Thanks ❤️ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🔗 Linked issue
N/A - discussed on Discord
❓ Type of change
📚 Description
The netlify preset was already using
preferStatic: truewhich lets existing static assets take precedence over the function, but this now also avoids a function invocation for static paths that don't exist, i.e. this avoids an unnecessary dynamic 404 that could be served directly from the CDN.The
netlify-edgepreset wasn't excluding anything, so this addresses both the 404 case and the existing asset case.The 404 case is important because browsers frequently attempt to request hashed assets from previous deploys that have been invalidated. There's no reason for this to go through functions, since we know that the whole parent path is static.
This PR also adds tests for the
netlify-edgepreset, which were missing. They're mostly copied from thenetlifypreset tests.📝 Checklist