sveltejs / kit

web development, streamlined
https://kit.svelte.dev
MIT License
18.56k stars 1.91k forks source link

adapter-static has issues with server endpoints #7183

Closed johnnysprinkles closed 1 year ago

johnnysprinkles commented 2 years ago

Describe the bug

One last issue on my big SvelteKit 350 to 551 upgrade... I'm using adapter-static to spit out some HTML files and serving those statically, some of those are even dynamic routes and that works fine with export const prerender = true.

The issue I'm having though is, I have a backend proxy I only use in development mode. You can see it added to my repro case in this commit: https://github.com/johnnysprinkles/sveltekit_static_dynamic/commit/e91dbdf61d410d817988b06c890513c76bbd6376 In that example it returns a simple string, but in real life is make an HTTP fetch to my actual backend.

With that in place, "npm run build" ends up failing with this message:

> Using @sveltejs/adapter-static
  @sveltejs/adapter-static: all routes must be fully prerenderable (unless using the 'fallback' option — see https://github.com/sveltejs/kit/tree/master/packages/adapter-static#spa-mode). Try adding `export const prerender = true` to your root +layout.js/.ts file — see https://kit.svelte.dev/docs/page-options#prerender for more details
    - src/routes/api/[...rest]
error during build:
Error: Encountered dynamic routes
    at adapt (file:///Users/jpsimons/dev/sveltekit_static_dynamic/node_modules/@sveltejs/adapter-static/index.js:35:12)
    at adapt (file:///Users/jpsimons/dev/sveltekit_static_dynamic/node_modules/@sveltejs/kit/src/core/adapt/index.js:28:8)
    at Object.handler (file:///Users/jpsimons/dev/sveltekit_static_dynamic/node_modules/@sveltejs/kit/src/exports/vite/index.js:488:12)
    at async PluginDriver.hookParallel (file:///Users/jpsimons/dev/sveltekit_static_dynamic/node_modules/rollup/dist/es/shared/rollup.js:22632:17)
    at async Object.close (file:///Users/jpsimons/dev/sveltekit_static_dynamic/node_modules/rollup/dist/es/shared/rollup.js:23709:13)
    at async Promise.all (index 0)
    at async build (file:///Users/jpsimons/dev/sveltekit_static_dynamic/node_modules/vite/dist/node/chunks/dep-db16f19c.js:45667:13)
    at async CAC.<anonymous> (file:///Users/jpsimons/dev/sveltekit_static_dynamic/node_modules/vite/dist/node/cli.js:748:9)

Is there a way to make the "dynamic route" check logic ignore server endpoints?

Reproduction

The repro is in https://github.com/johnnysprinkles/sveltekit_static_dynamic

If you sync the latest version of that, you can "npm run dev" and everything works using the backend proxy. But "npm run build" fails. It seems like adapter static should just ignore any +server.js files.

Logs

No response

System Info

System:
    OS: macOS 12.2.1
    CPU: (8) arm64 Apple M1
    Memory: 141.84 MB / 8.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 18.10.0 - ~/.nvm/versions/node/v18.10.0/bin/node
    npm: 8.19.2 - ~/.nvm/versions/node/v18.10.0/bin/npm
  Browsers:
    Firefox: 105.0.1
    Safari: 15.3
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.80
    @sveltejs/adapter-static: ^1.0.0-next.44 => 1.0.0-next.44
    @sveltejs/kit: next => 1.0.0-next.511
    svelte: ^3.44.0 => 3.50.1
    vite: ^3.1.0 => 3.1.6

Severity

blocking an upgrade

Additional Information

No response

thegamerx1 commented 2 years ago

This happens with the skeleton project as well, with just adding adapter static.

jamesb93 commented 2 years ago

I get this with a project with no dynamic routes and just a single +page.svelte

gbkwiatt commented 2 years ago

Same same, happens everywhere and it does not take in account any prerender = false or even config.kit.prerender.enabled = false

dummdidumm commented 2 years ago

The issue I'm having though is, I have a backend proxy I only use in development mode. You can see it added to my repro case in this commit: johnnysprinkles/sveltekit_static_dynamic@e91dbdf In that example it returns a simple string, but in real life is make an HTTP fetch to my actual backend.

To clarify: you want to make SvelteKit ignore the endpoints, because after deployment there's an API at the given URL?

This happens with the skeleton project as well, with just adding adapter static.

I get this with a project with no dynamic routes and just a single +page.svelte

This is expected. You need to add export const prerender = true or else nothing is prerendered, as the error message suggests.

Same same, happens everywhere and it does not take in account any prerender = false or even config.kit.prerender.enabled = false

This needs more info. Also, config.kit.prerender.enabled was removed a while ago in favor of setting this inside +layout/page(.server).js.

gbkwiatt commented 2 years ago

The issue I'm having though is, I have a backend proxy I only use in development mode. You can see it added to my repro case in this commit: johnnysprinkles/sveltekit_static_dynamic@e91dbdf In that example it returns a simple string, but in real life is make an HTTP fetch to my actual backend.

To clarify: you want to make SvelteKit ignore the endpoints, because after deployment there's an API at the given URL?

This happens with the skeleton project as well, with just adding adapter static.

I get this with a project with no dynamic routes and just a single +page.svelte

This is expected. You need to add export const prerender = true or else nothing is prerendered, as the error message suggests.

Same same, happens everywhere and it does not take in account any prerender = false or even config.kit.prerender.enabled = false

This needs more info. Also, config.kit.prerender.enabled was removed a while ago in favor of setting this inside +layout/page(.server).js.

The thing is that I have prerender=true in root +layout.js And all routes I don't want to prerender I have +server.js export const prerender=false but I still get error either saying that it was marked as prerender but it wasn't prerendered or as above all routes must be fully prerenderable

and my +server.js only has a GET method. So I am really confused, as no matter what I do I can't build with adapter-static. Either I don't umnderstand documentation correctly or ... I don't know

johnnysprinkles commented 2 years ago

That's what I was thinking (first comment). The endpoint in local development is super convenient. Without that I think I'd either have to prefix all my API calls and probably deal with CORS, or else put a proxy in front and split traffic between dev Vite and my real API.

Isn't the idea of prerendering an endpoint sort of nonsensical? Instead of saying "the fact that nonsensical to prerender things exist, you can't use adapter-static" I'm just wondering if it makes more sense to ignore the nonsensical to prerender things.

Rich-Harris commented 2 years ago

I'm lost. You have pages with dynamic data, but you're trying to prerender them?

gbkwiatt commented 2 years ago

I'm lost. You have pages with dynamic data, but you're trying to prerender them?

In my case I am trying not to prerender then but i either get the message with prerender false, that it encouraged dynamic route and I should try setting layout prerender true, but if I set it to true, it says it wont prerender it. So either I've missed something in docs or there is some bugs. Or maybe I am completely missing the point, but with whatever setting, I cant buils with adapter static.

Ive set global +layout.js with prerender true, and tried to set dynamic routes with prerender false. In all honesty I've tried all combinations, and I just can't build with static adapter.

dummdidumm commented 2 years ago

You can't use adapter static and use dynamic endpoints. There is no server running on the backend, so there is nothing that can serve that GET request. You need to use a different adapter for that.

@johnnysprinkles case is different, he wants that endpoint only during dev mode but ignore it when building.

gbkwiatt commented 2 years ago

@johnnysprinkles case is different, he wants that endpoint only during dev mode but ignore it when building.

Well my case is similar, I have 2 environments, one uses node adapter and that node environment builds production with adapter static. So yeah i want to use dynamic ones for node env, but not on production with adapter static.

johnnysprinkles commented 2 years ago

@Rich-Harris Yes, I have pages with dynamic data but since I'm static rendering the whole site, that data would be loaded client-side. You might wonder why I'm not just using an SPA style fallback in that case. Because, I like to prerender the skeleton of each individual page, as much as can be rendered without having any data, to get those appearing instantly before any hydration happens. Better experience, less spinners.

@dummdidumm Correct. It does seems like kind of an edge case I realize. At the moment I think my best workaround is to recursively rename s/+server.js/server.js/ as part of the "build" script step, then rename it back after. (And I thought those plus signs were pointless! Maybe they do have a good use.)

Rich-Harris commented 2 years ago

@gbkwiatt adapter-node will serve prerendered pages too. You probably don't need to use adapter-static separately.

@johnnysprinkles as an aside, it sounds like this will result in flashes of missing data as you navigate around the app — might be better to do the loading in load, but only if browser === true.

How many proxy endpoints are we talking about? Would it make sense to implement them via handle, rather than creating +server.js routes that will fall afoul of the 'this cannot be prerendered' rule?

gbkwiatt commented 2 years ago

@gbkwiatt adapter-node will serve prerendered pages too. You probably don't need to use adapter-static separately.

I am afraid I can't use node on production environment and that's where the problem is. Not sure why it used to be possible and worked fine and now it's not. But that just means I will need some script to remove dynamic routes before using adapter static, but also keep node environment still running. It is a bit of edge case, but I am in a bit of an impass now.

johnnysprinkles commented 2 years ago

@Rich-Harris There's only one, so yeah, turns out I can make it a hook. See latest commit at https://github.com/johnnysprinkles/sveltekit_static_dynamic/commit/96852f58a344951b89c13eaf2cfa2b0d2c1ae72b

I think we should leave this issue open though just because an endpoint leveraging the routing system would be nicer and more elegant.

And yeah, for that aside, I can explore doing the loading in load. Based on this it seems like onMount is a pretty good place to do it though.

Screen Shot 2022-10-10 at 4 34 47 PM
JackPriceBurns commented 2 years ago

@Rich-Harris @gbkwiatt Think there is some confusion with how we are using adapater-static here. We've been using adapter-static for over a year now and without problems until the recent changes.

We have normal page routes like /jacks-page/+page.svelte and on these pages we have simple get requests to some of our server routes. For example we have a server route to handle all of our images (as they're stored on a CMS) so we have /images/[...slug]/+server.js so when /jacks-page requests /images/homepage.jpg it gets handled by the server route and produces a jpg file which can be downloaded and stored when crawling and statically generating the site.

We have many of these server routes like this, some which produce images, some produce json files. We obviously don't want these server routes to be directly prerendered themselves because that doesn't make sense. However, we require them to be present and running when the crawler is statically generating the site.

So with the new changes to the routing system, how would we achieve what we were doing before? We've tried converting our old server routes to the new +server.js format and adding export const prerender = false; to the top, however adapter static tries prerendering them anyway (which fails) and seems to ignore the prerender option.

Your comment earlier suggested we need to use a different adapter instead of adapter-static. We're open to picking another one as long as the end result is a fully statically generated site, which adapter are you recommending?

The reason we're using a sveltekit route to proxy requests to an external image source is because we want to store those images in the statically generated site, and deploy that to a CDN (we use CloudFlare). This way we don't have to worry about our CMS going down or anything like that.

Rich-Harris commented 2 years ago

We obviously don't want these server routes to be directly prerendered themselves because that doesn't make sense

Why not? It sounds like prerendering them is exactly what you want to do.

gbkwiatt commented 2 years ago

but how do you prerender something like some.json/+server.js where it resolves to json response ?

import { json } from '@sveltejs/kit';

export async function GET() {
    return json({ message: 'OK' });
}

If I mark this one as to prerender I get Error: The following routes were marked as prerenderable, but were not prerendered:

sounds like you have something in mind that we obviously missed

dummdidumm commented 2 years ago

I encountered this situation in a Discord question. There the problem was (and maybe for you, too) that the server endpoint never was called, so it didn't turn up in the list of prerendered things. This produces a dead lock because either there's an error about the page not being prerendered or an error about a dynamic route being present. To get out of it you either need to remove that server endpoint or use another adapter, because the whole point of adapter-static is that there's no server runtime, just some html/js/css files lying around on a static file server.

gbkwiatt commented 2 years ago

Well it all makes sense, but in our case we need both at some point, have a static site and have a node site with dynamic routes. Which basically comes to conclusion, that upon building static site, we have to remove those dynamic routes, build it, and put them back.

Why does it have to be like this ? Why it can't just omit those marked as prerender = false ?

Rich-Harris commented 2 years ago

Because that's a much bigger footgun and source of confusion. It might seem silly to you, but people used to constantly ask why their POST handlers don't work on GitHub Pages, etc.

adapter-static is for static sites. It's possible that we could add an iKnowWhatImDoing: true option, but honestly it sounds like you'd be better off using adapter-node and copying the build/client and build/prerendered directories to where you need them.

JackPriceBurns commented 1 year ago

Because that's a much bigger footgun and source of confusion. It might seem silly to you, but people used to constantly ask why their POST handlers don't work on GitHub Pages, etc.

adapter-static is for static sites. It's possible that we could add an iKnowWhatImDoing: true option, but honestly it sounds like you'd be better off using adapter-node and copying the build/client and build/prerendered directories to where you need them.

But adapter-node doesn't produce a static site? So we'd then have to write our own custom code for converting that?

Considering we could do this before and we've now lost functionality, and seemingly the only option to get it back is to either completely change the way we're hosting our site (not an option) or to rewrite the functionality of adapter-static for our needs (which is also not ideal at all) Then surely an iKnowWhatImDoing: true option would be the best idea at the moment?

jamesb93 commented 1 year ago

I'm thoroughly confused at this point. I have a site with completely static content (not a single .js file) and adapter-static still complains that that the routes must be "fully prerenderable".

gbkwiatt commented 1 year ago

I'm thoroughly confused at this point. I have a site with completely static content (not a single .js file) and adapter-static still complains that that the routes must be "fully prerenderable".

I found out that for some reason even that I have +layout.js in a root with prerender true, I still have to mark some of the routes as prerender true with separated layout.js inside that route. Also it could have something to do with option in a config config.kit.prerender.entries to be honest it's more of a try and fail game.

dummdidumm commented 1 year ago

A minimum reproducible would be great

jamesb93 commented 1 year ago

A minimum reproducible would be great

https://github.com/jamesb93/static-site-repro

This produces the error if you pnpm run build but works with pnpm run dev

dummdidumm commented 1 year ago

That's expected, by default nothing is prerendered, you need to opt in to that, and the error message even tells you how - by adding export const prerender = true in a +layout.js you create at the root. What about the error message was unclear that you didn't do that?

dominikg commented 1 year ago

added +layout.js to jamesb93 repro and also a route /api/foo.json thats loaded in onMount of the page. running npm run dev works, but npm run build does not, throwing that the foo.json route is not prerenderable, despite root +layout.js exporting true.

https://stackblitz.com/edit/github-uwdjbe

gbkwiatt commented 1 year ago

added +layout.js to jamesb93 repro and also a route /api/foo.json thats loaded in onMount of the page. running npm run dev works, but npm run build does not, throwing that the foo.json route is not prerenderable, despite root +layout.js exporting true.

https://stackblitz.com/edit/github-uwdjbe

From what I've noticed +server.js routes have to be specifically marked for prerender. So you have to add export const prerender = true in +server.js

Not sure why is that, since I was under impression, if you add +layout.js with prerender true, it will prerender all routes regardless (or will try at least). But maybe it's because it treats +server.js as dynamic routes by default

Rich-Harris commented 1 year ago

It's all there in the docs. +server.js files aren't affected by layouts, which are a page concept: https://kit.svelte.dev/docs/page-options#prerender-prerendering-server-routes

jamesb93 commented 1 year ago

I'm still baffled by the change, not in terms of design but what I need to do to make it work "as before". I don't use any fancy databases or anything with Svelte and basically use it like a static site generator. There isn't a single bit of dynamic content that can't be pre-rendered AFAIK.

Can someone use my website as a learning lesson for me and other people on how to make it all work just fine with the static adapter?

https://github.com/jamesb93/my-website

dummdidumm commented 1 year ago

You commented already on this thread, and my response to that is the solution:

That's expected, by default nothing is prerendered, you need to opt in to that, and the error message even tells you how - by adding export const prerender = true in a +layout.js you create at the root.

Did you try that yet? I also asked you some questions, so I'm asking them again:

I don't mean to come across rude, I'm just curious.

jamesb93 commented 1 year ago

export const prerender = true

That's fine. I understand you're trying to understand it from the user perspective.

I did try your suggestion of adding a +layout.js. There are two issues with this in reverse chronological order.

1) I'm not even sure how that would fix things, and it doesn't. 2) The error message doesn't suggest its not trying to prerender them. It is complaining that the routes are not fully prerenderable. I struggle to understand why this is the case.

@sveltejs/adapter-static: all routes must be fully prerenderable (unless using the 'fallback' option — see https://github.com/sveltejs/kit/tree/master/packages/adapter-static#spa-mode). Try adding export const prerender = true to your root +layout.js/.ts file — see https://kit.svelte.dev/docs/page-options#prerender for more details.

For example, one of the routes that it whinges about is literally just this:

<script>
    import { goto } from '$app/navigation';
    import { onMount } from 'svelte';

    onMount(async () => [goto('https://www.github.com/jamesb93/ftis')]);
</script>

and another...

<a href='/patches/samplebrain_max.maxpat' download>samplebrain max experimentation</a>
dummdidumm commented 1 year ago

You are on a rather old version of @sveltejs/kit (430, the latest is 516) which doesn't play well with the latest version of the adapter.

gbkwiatt commented 1 year ago

I just want to add, that after thinking all this through I do agree that adpater-static should just deal with renderable routes, and not to to prerender server routes. (on some cases it actually works if you mark server route for prerender).

However it would be really good addition, if we could mark routes to be not picked up at all by adapter. Currently I have a script that copied only prerendable routes and build with adapter static with those routes. But it would be great if I could just mark route for not being picked up by build at all. I understand it's just my own use case, since I have to build 2 times, once with adapter-node and once with adapter-static.

Other than that I think it's all clear to me now.

jamesb93 commented 1 year ago

You are on a rather old version of @sveltejs/kit (430, the latest is 516) which doesn't play well with the latest version of the adapter.

  • bump @sveltejs/kit to 1.0.0-next.516
  • remove default: true from your svelte.config.js
  • create src/routes/+layout.js and add export const prerender = true to it

Ah. I forgot that I pinned the version of kit and so pnpm upgrade was actually not bumping the package.

Thanks!

Rich-Harris commented 1 year ago

But adapter-node doesn't produce a static site? So we'd then have to write our own custom code for converting that?

As mentioned in the previous comment, the static parts of the site are written to build/client and build/prerendered when you use adapter-node.


As of #7264 there's a more comprehensive error message, and adapter-static has a strict: false message which is a less tongue-in-cheek version of the iKnowWhatImDoing option. I don't recommend its use, but it should help in the cases where you're colouring outside the lines.

johnnysprinkles commented 1 year ago

Just to follow up here, I'm using strict: false and it's helpful, I reverted my hook.js file and restored my ... rest route for the API proxy in dev mode.

johnnysprinkles commented 1 year ago

Also, another follow-up, I just switched to using Vite's built-in SSL capabilities with something like this in my vite.config.js

const config = {
  server: {
    proxy: {
      '/api': {
        target: process.env.VITE_DEV_PROXY || 'http://localhost:8080',
        secure: false,
        changeOrigin: true,
      }
    },
   ...

So no need for either a dev-only [...rest] route, nor the hooks.js file, and the issue is moot for me.

buzzy commented 8 months ago

Because that's a much bigger footgun and source of confusion. It might seem silly to you, but people used to constantly ask why their POST handlers don't work on GitHub Pages, etc.

adapter-static is for static sites. It's possible that we could add an iKnowWhatImDoing: true option, but honestly it sounds like you'd be better off using adapter-node and copying the build/client and build/prerendered directories to where you need them.

This sounds like something I need. I am using capacitorJS that bundles in a javascript "front-end" together with a webview in a native Android/iOS app. Thus, I only need the "frontend"-part of the build, so that I can embed that into the capacitorjs app on the phone. Then I need the "backend"-part to run on a server. Currently, I am using adapter-node to create the backend (and frontend) build that I run on the server. But if I then use adapter-static, it's not able to use things like FormActions. What is the correct way to solve this? Thus not serving the front-end from node, but rather separate the 2 to be hosted separately. If I do it the way you describe, how would I configure the base-url that the front-end part should use? Thanks!