Simply having a `+page.server.ts` page crashes static builds

NatoBoram commented 1 year ago

Describe the bug

500 Internal Error

Reproduction

Create a src/routes/+page.server.ts file with this content

import type { PageServerLoad } from './$types';

export const load = (async () => {
    return;
}) satisfies PageServerLoad;

Make a build with adapter-static
Open it with http-server

Repro: https://github.com/NatoBoram/bug-report-sveltekit-ssr-csr

Logs

GET
http://127.0.0.1:8081/
[HTTP/1.1 404 Not Found 1ms]

TypeError: can't access property "then", a.default.detectStore(...) is undefined
h1-check.js:1:1301
XHRGET
http://127.0.0.1:8081/__data.json?x-sveltekit-invalidated=01
[HTTP/1.1 404 Not Found 1ms]

SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the JSON data app.306aa39f.js:1:5729

System Info

System:
    OS: Linux 6.2 Pop!_OS 22.04 LTS
    CPU: (12) x64 AMD Ryzen 5 5600X 6-Core Processor
    Memory: 12.92 GB / 31.26 GB
    Container: Yes
    Shell: 5.8.1 - /usr/bin/zsh
  Binaries:
    Node: 20.4.0 - /usr/bin/node
    npm: 9.8.0 - ~/.local/share/pnpm/npm
    pnpm: 8.6.6 - ~/.local/share/pnpm/pnpm
  Browsers:
    Chrome: 114.0.5735.198
  npmPackages:
    @sveltejs/adapter-auto: ^2.1.0 => 2.1.0 
    @sveltejs/adapter-node: ^1.3.1 => 1.3.1 
    @sveltejs/adapter-static: ^2.0.2 => 2.0.2 
    @sveltejs/kit: ^1.20.4 => 1.22.1 
    svelte: ^4.0.0 => 4.0.5 
    vite: ^4.3.6 => 4.4.1

Severity

Blocks all usage of SvelteKit for apps that need both adapter-node and adapter-static

Additional Information

The workflow I was aiming for is:

Catch a locale cookie in +layout.server.ts
In +layout.ts, return either the server's cookie or the browser's cookie
In +layout.svelte, setup a store with the cookie, put it in a context, use that context to translate the app

Because of that, it seems impossible to make SSR + CSR apps that have authentication via cookies; you either have to completely disable SSR or completely disable static builds, which is unacceptable.

When I read Adapter static assumes you have completely static output, no node server running, I understand that the rendered web page will assume you have completely static output, no node server running. But in reality, it doesn't actually assume that because a request to __data.json?x-sveltekit-invalidated=01 was made.

Not only that, but there's a whole section about when not to pre-render

Note Not all pages are suitable for prerendering. Any content that is prerendered will be seen by all users. You can of course fetch personalized data in onMount in a prerendered page, but this may result in a poorer user experience since it will involve blank initial content or loading indicators.

Accessing url.searchParams during prerendering is forbidden. If you need to use it, ensure you are only doing so in the browser (for example in onMount).

So it seems like pre-rendering is not suitable to applications with user authentification.

Essentially, what I'm saying is that adapter-static should behave exactly as if you were running this:

find src/routes -name '+*.server.*' -delete
pnpm build
http-server ./build
git checkout -- 'src/routes/+*.server.*'

And even if you were using data in +page.ts, it would just come out as null and you would be able to account for that and have an application that you can deploy on both a server with SSR and on a static file host like GitHub Pages.

But really, the elephant in the room is that simply having a +page.server.ts file shouldn't crash the page. Requests to __data.json?x-sveltekit-invalidated=01 should be completely disabled in static builds.

nabe1653 commented 1 year ago

In my project, layout.server.ts has only below options with adapter-static:

export const prerender = true;
export const csr = true;
export const ssr = false;
export const trailingSlash = 'always';

It works on svelte v3 and I created this for some reason at that time. But I faced __data.json loading after update v4 so searched this issue.

e3ndr commented 1 year ago

In my project, layout.server.ts has only below options with adapter-static:
export const prerender = true;
export const csr = true;
export const ssr = false;
export const trailingSlash = 'always';
It works on svelte v3 and I created this for some reason at that time. But I faced __data.json loading after update v4 so searched this issue.

I was able to fix this issue by exporting those options in my layout.js instead of layout.server.js. So it indeed seems that .server files are causing the calls to __data.json with adapter-static.

pzerelles commented 1 year ago

You can't use non-prerenderes .server files with adapter static. Adapter static assumes you have completely static output, no node server running. We should make that more obvious through a clear error message.

I tried to do that to import data for example from the filesystem. In page.ts, I cannot import modules that only work on NodeJS, in page.server.ts I can. Also, in the load function of page.ts, the data property mentions that this comes from either page.server.ts or layout.server.ts (does not work with layout.server.ts though).

I can accomplish the same by creating a server endpoint that is called with fetch from page.ts. But just using page.server.ts seemed simpler.

Is this something that should work or should page.server.ts be avoided when using adapter-static?

NatoBoram commented 1 year ago

Something like this works (on Linux):

FOUND=$(find src/routes -name '+*.server.*')
echo $FOUND | xargs rm

BUILD_ADAPTER='static' pnpm run build
echo $FOUND | xargs git checkout --

svelte-kit sync

You basically delete the +*.server.* files, make the static build, then restore them. This makes SvelteKit usable in both server-side deployments and client-side deployments from a single code base.

pzerelles commented 1 year ago

But the .server. files are important for the static build, too. They are used during SSR and collect data from the filesystem.

pzerelles commented 1 year ago

If I use adapter-static, SSR generates the HTML pages that are hydrated into an interactive client-side-rendered (CSR) page in the browser. During the build, the .server. files are used.

eltigerchino commented 1 year ago

Essentially, what I'm saying is that adapter-static should behave exactly as if you were running this:
find src/routes -name '+*.server.*' -delete
pnpm build
http-server ./build
git checkout -- 'src/routes/+*.server.*'
And even if you were using data in +page.ts, it would just come out as null and you would be able to account for that and have an application that you can deploy on both a server with SSR and on a static file host like GitHub Pages.

But really, the elephant in the room is that simply having a +page.server.ts file shouldn't crash the page. Requests to __data.json?x-sveltekit-invalidated=01 should be completely disabled in static builds.

These requests are required. The output of the server load function is saved during the prerender process as a __data.json file. This data file, along with the javascript to render the page, is requested during client-side rendering.

I'll close this because I can't reproduce the original issue. It throws an error during build if there is a non-prerendered server load function, rather than during the build preview (added in https://github.com/sveltejs/kit/pull/7264).

> Using @sveltejs/adapter-static
  @sveltejs/adapter-static: all routes must be fully prerenderable, but found the following routes that are dynamic:
    - src/routes/

  You have the following options:
    - set the `fallback` option — see https://kit.svelte.dev/docs/single-page-apps#usage for more info.
    - add `export const prerender = true` to your root `+layout.js/.ts` or `+layout.server.js/.ts` file. This will try to prerender all pages.
    - add `export const prerender = true` to any `+server.js/ts` files that are not fetched by page `load` functions.

    - pass `strict: false` to `adapter-static` to ignore this error. Only do this if you are sure you don't need the routes in question in your final app, as they will be unavailable. See https://github.com/sveltejs/kit/tree/master/packages/adapter-static#strict for more info.

  If this doesn't help, you may need to use a different adapter. @sveltejs/adapter-static can only be used for sites that don't need a server for dynamic rendering, and can run on just a static file server.
  See https://kit.svelte.dev/docs/page-options#prerender for more details

NatoBoram commented 1 year ago

You can test this by cloning git@github.com:NatoBoram/bug-report-sveltekit-ssr-csr.git. I have updated dependencies to their latest and the issue is still present.

There are further instructions in the README.md to make it run with the current issue or with a workaround.

// +page.server.ts
import type { PageServerLoad } from './$types';

export const load = (async ({ cookies }) => {
    const locale = cookies.get('locale');
    return { locale };
}) satisfies PageServerLoad;

// +page.ts
import { browser } from '$app/environment';
import cookie from 'cookie';
import type { PageLoad } from './$types';

export const load = (({ data }) => {
    if (browser)
        return {
            locale: cookie.parse(document.cookie)['locale'],
            from: 'browser'
        };

    return {
        locale: data.locale,
        from: 'server'
    };
}) satisfies PageLoad;

<!-- +page.svelte -->
<script lang="ts">
    import cookie from 'cookie';
    import type { PageData } from './$types';

    export let data: PageData;

    function setLocaleCookie() {
        const localeCookie = cookie.serialize('locale', 'fr', {
            domain: location.hostname,
            path: '/',
            sameSite: true,
            secure: location.protocol === 'https:'
        });
        console.log('Saving cookie:', localeCookie);
        document.cookie = localeCookie;
    }
</script>

<p><code>locale</code> cookie: <code>{data.locale}</code></p>

<p><code>locale</code> obtained from: <strong>{data.from}</strong></p>

<button on:click={setLocaleCookie}>Set <code>locale</code> cookie</button>

> Using @sveltejs/adapter-static
  Wrote site to "build"
  ✔ done
✓ built in 2.38s

GET
http://127.0.0.1:8080/
[HTTP/1.1 404 Not Found 3ms]

XHRGET
http://127.0.0.1:8080/__data.json?x-sveltekit-trailing-slash=1&x-sveltekit-invalidated=01
[HTTP/1.1 404 Not Found 1ms]

SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the JSON data [app.6b60deac.js:1:5717](http://127.0.0.1:8080/_app/immutable/entry/app.6b60deac.js)

eltigerchino commented 1 year ago

Ah you’re right. Sorry I missed that. We should probably throw an error during a static build when +page.server.js is exported without being prerendered

NatoBoram commented 1 month ago

Rather than throwing an error, I'd appreciate if it simply didn't make network requests and if data was null so one wouldn't have to rip out files from the project to build it for different environments.

eltigerchino commented 1 month ago

I'm not sure if a silent error is optimal here. Someone might want the data but forgot to add a export const prerendered = true to the file. We could do a check and if prerendered is omitted, then throw an error. But it if it's set to false based on an environment variable, etc. we could choose not to error?

NatoBoram commented 1 month ago

I like that solution.

This way, if I set export const prerender = false but still use adapter-static, then I'm clearly saying that the data from that +page.server.ts or +layout.server.ts shouldn't exist at runtime and to not fetch it. In my +layout.ts or +page.ts, data should be undefined or null and I should be able to continue as normal.

Someone could still enable pre-rendering on these files by simply adding export const prerender = true, in which case data should return whatever was done during build time.

sveltejs / kit