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

$page.query.get(..) returns null for prerendered pages #669

Closed
falsetru opened this issue Mar 25, 2021 · 12 comments · Fixed by #2104
Closed

$page.query.get(..) returns null for prerendered pages #669

falsetru opened this issue Mar 25, 2021 · 12 comments · Fixed by #2104
Labels
bug Something isn't working
Milestone

Comments

@falsetru
Copy link

falsetru commented Mar 25, 2021

Describe the bug
$page.query.get(...) returns null.

To Reproduce

git clone --branch issue-1 https://github.com/falsetru/sveltekit-test.git issue-1
cd issue-1
npm install
npm run build
npx sirv build/  # `npm run start` will hide the issue.

Expected behavior
abc printed, but got null.

Information about your SvelteKit Installation:

Diagnostics
  • The output of npx envinfo --system --npmPackages svelte,@sveltejs/kit,vite --binaries --browsers
  System:
    OS: Linux 5.8 Ubuntu 20.10 (Groovy Gorilla)
    CPU: (4) x64 Intel(R) Core(TM) i7-7600U CPU @ 2.80GHz
    Memory: 763.02 MB / 15.39 GB
    Container: Yes
    Shell: 5.8 - /usr/bin/zsh
  Binaries:
    Node: 12.18.2 - /usr/bin/node
    npm: 7.7.6 - ~/.npm-packages/bin/npm
  Browsers:
    Chrome: 89.0.4389.114
    Firefox: 87.0
  npmPackages:
    @sveltejs/kit: next => 1.0.0-next.67 
    svelte: ^3.29.0 => 3.37.0 
    vite: ^2.1.0 => 2.1.5 
  • Your browser
    Google Chrome Version 89.0.4389.114 (Official Build) (64-bit)

  • Your adapter
    @sveltejs/adapter-static

Additional context

  • As a workaround, I am using new URLSearchParams(document.location.search).get('name').
  • If I modify
    query: new URLSearchParams(${s(request.query.toString())}),
    to
    query: new URLSearchParams(document.location.search),
    the issue is gone. Not sure whether it's proper way.
@andreasnuesslein
Copy link

andreasnuesslein commented Mar 30, 2021

I'm having the same problem 🖐️

@falsetru Your solution should only work within the browser because document is not defined outside the browser (i.e. when adapter-static is prerendering everything)

@benmccann

This comment has been minimized.

@andreasnuesslein

This comment has been minimized.

@falsetru
Copy link
Author

falsetru commented Apr 2, 2021

@andreasnuesslein I use document.location.search as a workaround, not a solution.

To avoid accessing document outside browser, you can use browser from $app/env.

import { browser } from '$app/env';

...


const qs = browser ? document.location.search : '';
const query = new URLSearchParams(qs);
const name = query.get('name') || 'unknown';

@BibbyChung
Copy link

@andreasnuesslein I use document.location.search as a workaround, not a solution.

To avoid accessing document outside browser, you can use browser from $app/env.

import { browser } from '$app/env';

...


const qs = browser ? document.location.search : '';
const query = new URLSearchParams(qs);
const name = query.get('name') || 'unknown';

yes.. if you use this solution, you should turn off [ssr: false] in svelte.config.cjs.
waiting the best solution...

@benmccann
Copy link
Member

This isn't just adapter-static, but any prerendered page. I also got it with adapter-node and export const prerender = true;

@benmccann benmccann changed the title $page.query.get(..) returns null, with adapter-static $page.query.get(..) returns null for prerendered pages May 26, 2021
@benmccann
Copy link
Member

benmccann commented May 26, 2021

The issue here is that a prerendered page is a page that has already been rendered and saved as HTML. Since it's been rendered, there's no way on the server-side to use the query. You'd have to disable prerendering if you want to use the query on the server.

Right now, it's pretty easy not to realize that there's an issue there. There are a few options:

  • prerendering could consider the URL as part of the page lookup key. This seems difficult though because users could potentially type any arbitrary query string and what happens if they type one that hasn't been prerendered
  • probably what I'd prefer is to have query be null or throw an exception if you try to use it during prerendering to make it more obvious that it's something you shouldn't do. If there's something you want to do with the query string you should do it using the location either in onMount or behind an if (browser) check
  • as fix: construct query params from location.search on rehydrate #1511 suggests we could just ignore the inability to render on the server-side and then fix up the content during hydration. This option makes me a bit nervous. While it would work and might be easier for the user, I think it also hides the fact that your content might not be what you expect when a search engine sees it. It also would not be compatible with possible additional hydration modes that could be introduced in the future. E.g. if you consider the way that React does hydration where the DOM content is not updated, but event listeners are simply added, then this solution would not work

update: I think Rich's thinking largely aligns with my proposed solution given his comment here: #1511 (comment)

@mzaini30
Copy link

I run this code after build:

import replace from 'replace'

replace({
	regex: 'query: new URLSearchParams(.+),',
	replacement: 'query: new URLSearchParams(location.search),',
	paths: ['build'],
	recursive: true
})

So, the problem gone~

@pzerelles
Copy link
Contributor

When the page is already hydrated, further navigation will update the $page store with the query params. Not having them available after hydration makes the $page store's query prop unusable for static sites, because navigation will lead to other results than reloads of the same url.

Of course one should be aware of that a prerendered page does not have query params for search engines.

Possible additional hydration modes could update the $page store on hydration with the query params.

The use of query params is of course not always the best way to handle things and it will never be optimal in all situations. Developer should know the implications, especially with prerendered pages. Giving warnings in the documentation with a couple of use cases could help there, too, without taking away the simplicity of having them available in the $page store.

@johnnysprinkles
Copy link
Contributor

johnnysprinkles commented May 29, 2021

@mzaini30 You probably saw my RFC sveltejs/rfcs#52 that I mentioned in the PR related to this issue. I did some exploration into web server interpolation and the last line of this commit has a replace similar to what you're doing, but I'm doing it at runtime instead of build time, simply replacing one static value with another. Your dynamic approach of putting code there instead of data seems like the right direction to me (and is compatible with hosting on a CDN, not needing a logicful web server). Feel free to comment in my RFC.

https://github.com/johnnysprinkles/static-all-the-things/commit/5ed2e708ec1a355788b8bfb9deb24ff70d51b8e2#diff-b1353b49842770945c9871cb80af62527bba83ce94402cd18a5e097fd6d3d7b9R19

html = f.read().decode("utf-8")
return html.replace("[id]\"", match[1] + "\"").encode("utf-8")

Of course just updating SvelteKit itself to do this would be the ideal route.

@mzaini30
Copy link

@johnnysprinkles So, can you generate src/routes/[id].svelte to /[id]/index.html? It's awesome~

@benmccann
Copy link
Member

This could be implemented by making page a proxy object that throws an Error if prerendering is enabled (it'd be nice to do it even in dev mode if the option is on so that the developer catches it faster):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
7 participants