Skip to content

CSR and SSR URL mismatch with rewrites #5093

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

Closed
phultquist opened this issue May 27, 2022 · 3 comments
Closed

CSR and SSR URL mismatch with rewrites #5093

phultquist opened this issue May 27, 2022 · 3 comments
Labels
bug Something isn't working help wanted PRs welcomed. The implementation details are unlikely to cause debate low hanging fruit p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc.
Milestone

Comments

@phultquist
Copy link

phultquist commented May 27, 2022

Describe the bug

It seems that the $page.url.pathname is incorrect on the server when using a rewrite — instead of returning the URL of the source URL, it returns that URL of the destination URL (but only on the server). This can cause CLS.

Here's a video demo of the issue.

Here's the deploy URL as a demo. Try hard reloading this page (notice /rewrite). The $page.url.pathname is different on the client and the server when rendering on the server. It causes the Current Path to flash.

For reference, Next will do the same thing. However, when developing locally, it throws a hydration error.

Reproduction

https://github.com/phultquist/svelte-ssr-pathname-bug

Note, when developing locally, use vercel dev instead of npm run dev

Logs

# $: { console.log($page.url.pathname) }

# Server

/

# Console

/rewrite

System Info

System:
    OS: macOS 12.3.1
    CPU: (10) arm64 Apple M1 Max
    Memory: 229.13 MB / 32.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.14.2 - ~/.nvm/versions/node/v16.14.2/bin/node
    Yarn: 1.22.17 - /usr/local/bin/yarn
    npm: 8.5.2 - ~/node_modules/.bin/npm
  Browsers:
    Chrome: 101.0.4951.64
    Safari: 15.4
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.46 
    @sveltejs/kit: next => 1.0.0-next.342 
    svelte: ^3.46.0 => 3.48.0

Severity

annoyance

Additional Information

I'm not well-versed to the Svelte repository — but it looks like it's coming from this line (if that helps at all 😄).

@Rich-Harris Rich-Harris added the bug Something isn't working label Jul 16, 2022
@Rich-Harris Rich-Harris added this to the 1.0 milestone Jul 16, 2022
@Rich-Harris Rich-Harris added the help wanted PRs welcomed. The implementation details are unlikely to cause debate label Jul 16, 2022
@benmccann benmccann added p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. low hanging fruit low hanging fruit and removed low hanging fruit labels Jul 19, 2022
@sphinxc0re
Copy link
Contributor

I don't know If I misunderstand rewrites in general or if vercels rewrites are special in any way but being able to know the original url on the server side seems pretty much impossible to me.

To clarify how I understand it: A rewrite should only send the "destination" request to the service and the service should not really know of the rewrite. This makes rewrites very flexible as you could mount your application anywhere without changing your business logic. The other way would be possible but I don't know how much work that would be: Getting the client-side to also only know of the destination url seems impractical to me as the server would have to dictate what the "correct" url is.

As I said: I'm unsure whether my knowledge of rewrites is just plain wrong so please correct me if that's the case. This is just my analysis so far. @phultquist

@sphinxc0re
Copy link
Contributor

I want to work on resolving this issue but from my analysis I don't know what the next course of action would be.

@Rich-Harris
Copy link
Member

Yeah, it's not at all clear how SvelteKit should handle this case. The URL is different between SSR and CSR, so SvelteKit's behaviour is arguably correct.

Moreover, even if the client render did respect the rewrite (probably fairly easy to do on initial render, we just pass the pathname from server to client, though IIRC we stopped doing that a while back for... some reason), you'd just be delaying the mismatch until the first client-side navigation that involved a rewrite.

In summary I think the best advice we can give is 'rewrites: don't'.

If we do support rewrites, it would need to be via SvelteKit-controlled config, similarly to how Next does it. There's an existing issue for that — #5703 — so I'll close this.

@Rich-Harris Rich-Harris closed this as not planned Won't fix, can't repro, duplicate, stale Sep 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted PRs welcomed. The implementation details are unlikely to cause debate low hanging fruit p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc.
Projects
None yet
Development

No branches or pull requests

4 participants