Skip to content
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

trailingSlash ignore gives wrong URL pathname #9595

Closed
GingerAdonis opened this issue Apr 3, 2023 · 11 comments · Fixed by #9738 or #10475
Closed

trailingSlash ignore gives wrong URL pathname #9595

GingerAdonis opened this issue Apr 3, 2023 · 11 comments · Fixed by #9738 or #10475
Labels
bug Something isn't working ready to implement please submit PRs for these issues!
Milestone

Comments

@GingerAdonis
Copy link
Contributor

Describe the bug

Viewing a page with a trailing slash when rendered clientside will give an URL pathname without the trailing slash in +page.server.js. This is quite problematic for custom trailingSlash handling in the load function.

Example:

  • /myroute/ server load gives pathname: /myroute/ (ok)
  • /myroute/ client load gives pathname: /myroute (not ok)

Reproduction

Please note that trailingSlash is set to ignore. Click both links and see the problem when viewing a page with a trailing slash.

https://stackblitz.com/edit/sveltejs-kit-template-default-y3g4ee?file=src/routes/about/+page.svelte

Logs

No response

System Info

System:
    OS: Windows 10 10.0.22621
    CPU: (12) x64 AMD Ryzen 5 5600U with Radeon Graphics
    Memory: 2.41 GB / 13.86 GB
  Binaries:
    Node: 18.15.0 - C:\Program Files\nodejs\node.EXE
    npm: 9.6.3 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.22621.1413.0), Chromium (111.0.1661.62)
    Internet Explorer: 11.0.22621.1
  npmPackages:
    @sveltejs/adapter-node: 1.2.3 => 1.2.3
    @sveltejs/kit: 1.15.0 => 1.15.0
    svelte: 3.58.0 => 3.58.0
    vite: 4.2.1 => 4.2.1

Severity

serious, but I can work around it

Additional Information

No response

@GingerAdonis GingerAdonis changed the title trailingSlash ignore gives wrong URL pathname trailingSlash ignore gives wrong URL pathname Apr 4, 2023
@amit13k
Copy link

amit13k commented Apr 14, 2023

I am facing similar issue where client side navigation with a trailing slash doesn't contain trailing slash in the pathname in load function. As a workaround, I am just adding a trailing slash if it's not already present. Hoping to see this fixed sooner.

@dreitzner
Copy link
Contributor

dreitzner commented Apr 19, 2023

update

After some debugging if found out, that trailing_slash is always undefined.
The section of code that is responsible can be found here:
packages\kit\src\runtime\server\respond.js => respond() in line 177
trailing_slash is only set when it is not a data request, therefore undefined.


After some debugging, here is some more info:

  • render_page() returns correct URL
URL {
  href: 'http://billa.local:3000/about/',
  origin: 'http://billa.local:3000',
  protocol: 'http:',
  username: '',
  password: '',
  host: 'billa.local:3000',
  hostname: 'billa.local',
  port: '3000',
  pathname: '/about/',
  search: '',
  searchParams: URLSearchParams {},
  hash: ''
}
  • render_data() returns wrong url
URL {
  href: 'http://billa.local:3000/about',
  origin: 'http://billa.local:3000',
  protocol: 'http:',
  username: '',
  password: '',
  host: 'billa.local:3000',
  hostname: 'billa.local',
  port: '3000',
  pathname: '/about',
  search: '',
  searchParams: URLSearchParams {},
  hash: ''
}

adding to that:

  • export const trailingSlash = 'always'; is present in ./src/routes/+layout.js
  • I still get trailing_slash as never in render_data(), which is why the normalize_path does not work
  • I also tried adding the trailing slash option to the root server layout => same issue
  • I also tried to add a layout to the route, but it is still never

@dreitzner
Copy link
Contributor

@Rich-Harris the logic was introduced with PR #7719 .
Do you remember, why the trailing slash logic was excluded for data requests?
Link to original PR: https://github.com/sveltejs/kit/pull/7719/files#diff-70bbf390c5a53b49a9f23557426bbd1bee9188a247ce1ef34e5f4f217036c7f6R180
Link to current code block: https://github.com/sveltejs/kit/blob/master/packages/kit/src/runtime/server/respond.js#L178

For every client site routing the trailing slash logic is never called and it is therefore undefined, when calling render_data()

@dreitzner
Copy link
Contributor

@dummdidumm as reviewer of this PR, you also might be able to give more insight into the question above. 😊

@dummdidumm
Copy link
Member

dummdidumm commented Apr 20, 2023

I think we did that because data requests don't need to be redirected, but the logic seems flawed because while we don't want to redirect, we still want to compute the trailingSlash value (because right now it's always undefined as you say and then falls back to never). A PR changing that would be appreciated 👍

@dummdidumm dummdidumm added bug Something isn't working ready to implement please submit PRs for these issues! labels Apr 20, 2023
@dummdidumm dummdidumm added this to the soon milestone Apr 20, 2023
dummdidumm pushed a commit that referenced this issue Apr 25, 2023
fixes #9595
Trailing slash was never set for preloading data through someroute/__data.json
@GingerAdonis
Copy link
Contributor Author

GingerAdonis commented May 1, 2023

I've updated the stackblitz link to use kit v1.15.9 and this bug still occurs. Do you have any idea? @dummdidumm / @dreitzner :)

@dreitzner
Copy link
Contributor

@GingerAdonis that is weird....

when I create a new SvelteKit project and run it locally, I get the correct behavior.
See https://github.com/dreitzner/svelte-kit-trailing-slash

@GingerAdonis
Copy link
Contributor Author

when I create a new SvelteKit project and run it locally, I get the correct behavior. See https://github.com/dreitzner/svelte-kit-trailing-slash

Your repo has a +page.js file instead of a +page.server.js file to properly reproduce this bug.

@dreitzner
Copy link
Contributor

ok, that makes sense, there is currently no way to communicate to the server that there should be a trailing slash in preload, when it is set to ignore. I updated my test repo with /server and /server/

Example:
/server will call http://127.0.0.1:5173/server/__data.json?x-sveltekit-invalidated=_1
/server/ will call http://127.0.0.1:5173/server/__data.json?x-sveltekit-invalidated=_1

@GingerAdonis
Copy link
Contributor Author

@dummdidumm Can the issue be re-opened? Thank you. :)

@dummdidumm dummdidumm reopened this May 4, 2023
@GingerAdonis
Copy link
Contributor Author

GingerAdonis commented Jun 6, 2023

I'm kind-of stuck on this issue. Is there anyone that knows a (hacky) workaround for this issue? Thanks in advance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ready to implement please submit PRs for these issues!
Projects
None yet
4 participants