-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
fix: correctly restore trailing slash in url pathname for data requests #10475
fix: correctly restore trailing slash in url pathname for data requests #10475
Conversation
🦋 Changeset detectedLatest commit: af688ad The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! I think the idea makes sense to signal the server that the trailing slash should be kept. I'm not 100% sure we need a query param for this, or if we could just do .../data.json/
instead.
Could you also add a test for this?
I'd say that adding a trailing slash to the data request itself might unnecessarily complicate the structure of the data requests URLs. Another option would be to simply change I'll try to implement a test. :) |
I'm unsure how to approach a test for this. Could anyone point me in the right direction? |
Navigate to You can explore An example test could be fetching from a trailing slash endpoint and checking if it returns successfully using the expected URL structure. |
Thank you @s3812497 for the thorough explanation. I've added tests for this PR. However, these tests will fail until #10531 is merged. |
5e9963f
to
59e9a1a
Compare
59e9a1a
to
af688ad
Compare
I've included the changes from #10531 as requested. @dummdidumm This should be good to go. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Fixes #9595 and #10125.
Because
DATA_SUFFIX
is set to/__data.json
an existing trailing slash gets omitted in a data request. This makes it impossible to know if the request URL pathname has a trailing slash. This issue normally applies only to routes withtrailingSlash
set toignore
in combination with a server load function.This PR fixes this behaviour by adding a
x-sveltekit-trailing-slash
searchparam to data requests with a trailing slash so the server can restore the omitted trailing slash. This approach keeps theDATA_SUFFIX
the same to ensure backwards compatibility. This was the least intrusive way I found to fix the issue.Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm test
and lint the project withpnpm lint
andpnpm check
Changesets
pnpm changeset
and following the prompts. Changesets that add features should beminor
and those that fix bugs should bepatch
. Please prefix changeset messages withfeat:
,fix:
, orchore:
.