feat: add logging to understand impact of trailing slash 308s #2743
Conversation
This file contains hidden or 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
📊 Package size report 0.01%↑
Unchanged files
🤖 This report was automatically generated by pkg-size-action |
pieh
reviewed
Jan 20, 2025
| @@ -234,6 +235,13 @@ export const setCacheControlHeaders = ( | |||
| return | |||
| } | |||
|
|
|||
| // temporary diagnostic to evaluate number of trailing slash redirects | |||
| if (status === 308 && request.url.endsWith('/') !== nextConfig.trailingSlash) { | |||
There was a problem hiding this comment.
Should we perhaps also check if Location header and request.url pathnames are almost the same and only diff is that one of those would have trailing slash to limit potential false positives?
Conceptually there could other redirects that would fall into this condition that are not related to trailing slash redirects (tho not sure about exact details)
There was a problem hiding this comment.
Thanks, I thought about that, but then I couldn't think of any other redirects that would fall into this condition. It's impossible to configure user redirects that don't match the trailing slash settings and I couldn't think of any others. Did you have any specific in mind?
There was a problem hiding this comment.
Maybe redirects to external domains/targets could have this characteristic possibly next skipping trailing slash handling if target is external, but as I mentioned - I don't have any concrete scenario in mind, so we can skip this if this is already something you thought about
pieh
approved these changes
Jan 20, 2025
mrstork
added a commit
that referenced
this pull request
Mar 31, 2025
mrstork
added a commit
that referenced
this pull request
Apr 1, 2025
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.
Description
We are considering caching 308 trailing slash redirects to reduce function invocations.
This PR will allow us to see the impact of this change.
BEGIN_COMMIT_OVERRIDE
chore: add logging to understand impact of trailing slash 308s
END_COMMIT_OVERRIDE