-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
fix: remove unnecessary path validation which breaks fetch with custom path base #15291
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?
Changes from 7 commits
db812b3
eb708f3
7a1700b
8d450d9
295b2b6
5ec2276
038ec8a
1daafbd
208b2ab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@sveltejs/kit': major | ||
| --- | ||
|
|
||
| fix: `fetch` not working when URL is same host but different than `paths.base` | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,7 @@ | |
| fetch('/request-abort', { headers: { accept: 'application/json' } }).then( | ||
| async (r) => (result = await r.json()) | ||
| ); | ||
| }, 100); | ||
| }, 50); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why was this reduced?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test was flaky, it was failing randomly in the CI and locally, reducing the abort signal timing helped. Should I revert this change? |
||
| } | ||
|
|
||
| onMount(test_abort); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| export function GET() { | ||
| return new Response('root'); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| export async function load({ fetch }) { | ||
| const response = await fetch('/not-base-path/'); | ||
| return { | ||
| fetchUrl: response.url, | ||
| fetchResponse: await response.text() | ||
| }; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| <script> | ||
| /** @type {import('./$types').PageProps} */ | ||
| const { data } = $props(); | ||
| </script> | ||
|
|
||
| <p data-testid="fetch-url">{data.fetchUrl}</p> | ||
| <p data-testid="fetch-response">{data.fetchResponse}</p> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| export async function load({ fetch }) { | ||
| // fetch to root with trailing slash | ||
| const response1 = await fetch('/'); | ||
| // fetch to root without trailing slash | ||
| const response2 = await fetch(''); | ||
| // fetch to root with custom base path with trailing slash | ||
| const response3 = await fetch('/path-base/'); | ||
| // fetch to root with custom base path without trailing slash | ||
| const response4 = await fetch('/path-base'); | ||
|
|
||
| return { | ||
| fetchUrl1: response1.url, | ||
| fetchUrl2: response2.url, | ||
| fetchUrl3: response3.url, | ||
| fetchUrl4: response4.url, | ||
| fetchResponse1: await response1.text(), | ||
| fetchResponse2: await response2.text(), | ||
| fetchResponse3: await response3.text(), | ||
| fetchRedirect4: response4.headers.get('location') | ||
| }; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| <script> | ||
| /** @type {import('./$types').PageProps} */ | ||
| const { data } = $props(); | ||
| </script> | ||
|
|
||
| <h2>Fetch URLs</h2> | ||
|
|
||
| <dl> | ||
| <dt>fetch1-url</dt> | ||
| <dd data-testid="fetch1-url">{data.fetchUrl1}</dd> | ||
| <dt>fetch2-url</dt> | ||
| <dd data-testid="fetch2-url">{data.fetchUrl2}</dd> | ||
| <dt>fetch3-url</dt> | ||
| <dd data-testid="fetch3-url">{data.fetchUrl3}</dd> | ||
| <dt>fetch4-url</dt> | ||
| <dd data-testid="fetch4-url">{data.fetchUrl4}</dd> | ||
| </dl> | ||
|
|
||
| <h2>Fetch Responses</h2> | ||
| <dl> | ||
| <dt>fetch1-response</dt> | ||
| <dd data-testid="fetch1-response">{data.fetchResponse1}</dd> | ||
| <dt>fetch2-response</dt> | ||
| <dd data-testid="fetch2-response">{data.fetchResponse2}</dd> | ||
| <dt>fetch3-response</dt> | ||
| <dd data-testid="fetch3-response">{data.fetchResponse3}</dd> | ||
| <dt>fetch4-redirect</dt> | ||
| <dd data-testid="fetch4-redirect">{data.fetchRedirect4}</dd> | ||
| </dl> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| export function GET() { | ||
| return new Response('relative'); | ||
| } |
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.
It should only be major for a breaking change (of a previously valid behaviour?). However, I don't think anyone would purposely try to have the load fetch trigger the SvelteKit server response if it's requesting an entirely different path