Prerendered page alongside non-GET request handler should be an error

[Rich-Harris]

Describe the bug

Related to #3294. I don't think there's a situation in which it would make sense to have a non-GET request handler alongside a prerendered page.

In the same vein as #4093 (comment), SvelteKit should throw an error if a prerenderable page (either export const prerender = true, or config.kit.prerender.default = true without a export const prerenderable = false) has an endpoint with non-GET methods.

Reproduction

Add a non-GET method to any page endpoint and mark the page as prerenderable: https://stackblitz.com/edit/sveltejs-kit-template-default-8la3qs?file=src%2Froutes%2Fabout.js&terminal=dev

Logs

No response

System Info

System:
    OS: macOS 12.0.1
    CPU: (10) arm64 Apple M1 Max
    Memory: 169.98 MB / 32.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.13.1 - ~/.nvm/versions/node/v16.13.1/bin/node
    Yarn: 1.22.17 - ~/.nvm/versions/node/v16.13.1/bin/yarn
    npm: 8.1.2 - ~/.nvm/versions/node/v16.13.1/bin/npm
  Browsers:
    Chrome: 98.0.4758.109
    Firefox: 97.0.1
    Safari: 15.1
  npmPackages:
    svelte: ^3.43.0 => 3.44.2

Severity

annoyance

Additional Information

No response

[tadmccorkle]

@benmccann I was looking into this one - I might be misunderstanding the issue, but I think this is already implemented when building. The following can be observed with the StackBlitz link above by running npm run build.

Currently, building with either:

• a page with export const prerender = true; with a non-get endpoint
• config.kit.prerender.default = true and a page without export const prerender = false; with a non-get endpoint

results in the following error after Vite's build output:

Error: Cannot prerender pages that have endpoints with mutative methods
    at load_shadow_data (file:///[OMITTED]/kit-sandbox/.svelte-kit/output/server/index.js:1038:13)
    at async load_node (file:///[OMITTED]/kit-sandbox/.svelte-kit/output/server/index.js:828:28)
    at async respond$1 (file:///[OMITTED]/kit-sandbox/.svelte-kit/output/server/index.js:1241:22)
    at async resolve (file:///[OMITTED]/kit-sandbox/.svelte-kit/output/server/index.js:1584:105)
    at async Object.handle (file:///[OMITTED]/kit-sandbox/.svelte-kit/output/server/chunks/hooks-5b953b47.js:6:20)
    at async respond (file:///[OMITTED]/kit-sandbox/.svelte-kit/output/server/index.js:1540:22)
    at async visit (file:///[OMITTED]/kit/packages/kit/src/core/build/prerender/prerender.js:145:20)
> 500 /about
    at file:///[OMITTED]/kit/packages/kit/src/core/build/prerender/prerender.js:31:11
    at save (file:///[OMITTED]/kit/packages/kit/src/core/build/prerender/prerender.js:266:4)
    at visit (file:///[OMITTED]/kit/packages/kit/src/core/build/prerender/prerender.js:155:3)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

That error is thrown here: https://github.com/sveltejs/kit/blob/master/packages/kit/src/runtime/server/page/load_node.js#L389

So am I misunderstanding the issue and/or not seeing a flaw or gap in the current implementation? Or is there some other desired outcome from this issue, like a new test for this case or for the error to be thrown in a context other than when building?

[mrkishi]

@tadmccorkle I believe this is about displaying an error page during dev as well, as to minimize dev/build disparity.

[pto1026]

I would like to tackle this issue, can you please assign me

[tadmccorkle]

@pto1026 I tried looking into this one, so I wanted to let you know what didn't work for me - I probably just don't know the correct place to implement the fix.

I wanted to make the fix on the dev side of things, and the most appropriate place felt like the plugin.js file where the vite sveltekit plugin is defined. Adding a prerender property here (https://github.com/sveltejs/kit/blob/master/packages/kit/src/core/dev/plugin.js#L342-L348) got the error page to show if you refresh the browser on an offending page when the svelte config file specifies prerender by default, but it won't show upon navigation.

I tried adding logic to the server's respond function (https://github.com/sveltejs/kit/blob/master/packages/kit/src/runtime/server/index.js#L16), but had similar results - any changes I made only produced the error page on browser refresh, not a navigation.

I'm pretty new to the code base, so, again, I'm probably not looking in the right places. Either that, or the fix will touch more code than I was expecting.