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

adapter-cloudflare: serve prerendered pages and static assets using _routes.json #7298

Closed
colecrouter opened this issue Oct 17, 2022 · 14 comments

Comments

@colecrouter
Copy link

Describe the problem

From what I've seen and tested (please correct me), sites deployed via Cloudflare Pages still process prerendered pages (export const prerender = true;) and static assets through _worker.js. This means that even requests to static resources contribute to the host's "function calls" budget.

Describe the proposed solution

This recently updated Cloudflare doc indicates that _routes.json can be used to bypass _worker.js, which typically handles all requests. By restructuring the build output folder (to serve static assets as they are normally routed in-app), we could take advantage of this feature. This would make adapter-cloudflare infinitely more viable for high traffic sites.

Alternatives considered

  • Using adapter-static, but that only works if all of your site is prerendered.
  • Stricter rate limiting, but that's not scalable, reliable, or customizable (4 rules max on Cloudflare).
  • Maybe surprise outages are OK ¯\_(ツ)_/¯

Importance

i cannot use SvelteKit without it

Additional Information

I understand that traditionally, the contents of the build output folder is not meant to be exposed, and that _worker.js is meant to handle every request, even for static assets. However, I think that falling back onto Cloudflare Page's built-in file routing would be well worth it if it meant saving actual money on hosting costs (or in my case, having my site stay live for the whole day).

@colecrouter
Copy link
Author

colecrouter commented Oct 18, 2022

I've made a proof of concept. It uses a _redirects file to reroute prerendered pages to their outputted path (root level, I have not tested with nested prerendered routes). Aside from that, static assets are included and work as normal, with nothing weird, however it looks like it was intentionally not done this way. I'm assuming this has to do with having hooks that catch requests to static assets or prerendered pages. That's going to be a whole other can of worms...

Note that index.html does not work yet, as it appears Cloudflare Pages is broken there.

@MedLeon
Copy link

MedLeon commented Oct 31, 2022

This is a blocking issue for me as well.

I do not think that the _redirect file is the way to go. The whole concept of a monolith _worker.js file is just for migration and should be abondend at some point.
The "new" way would be to deploy a [[path]].js file. I would recommend the Cloudflare adapter for Astro. There you got two modes and one of them produces a [[path]].js file.

@colecrouter
Copy link
Author

I would recommend the Cloudflare adapter for Astro. There you got two modes and one of them produces a [[path]].js file.

That's good to know, I'll keep that in mind.

I do not think that the _redirect file is the way to go. The whole concept of a monolith _worker.js file is just for migration and should be abondend at some point. The "new" way would be to deploy a [[path]].js file.

I think this would require a very large breaking overhaul to how SvelteKit compiles. However, the _routes.json approach could work without redirects, if SvelteKit compiled a static page to /thing/index.html rather than /thing.html (ie. the outputted file goes to where you would expect to access it via URL).

@benmccann
Copy link
Member

I think static assets were addressed in #6530. Maybe prerendered pages weren't though?

@benmccann benmccann added this to the whenever milestone Nov 9, 2022
@colecrouter
Copy link
Author

I think static assets were addressed in #6530. Maybe prerendered pages weren't though?

Here, it looks like static assets are not (unless favicon). Check out the comment there, I'm not really sure what I would do in that case. On the other hand, prerendered pages wouldn't work anyway, because they're not being outputted to the right folder/name.

In my fork, I worked around the assets (literally just comment the line), but working around the prerendered pages is janky.

In short, we could add static assets, but that would disregard the comment made by the Pages crew. We could add prerendered pages, but in a way that's janky/messes with stuff.

@colecrouter
Copy link
Author

I've redone my branch and modified how static pages are outputted. Interestingly, it seems like they were intentionally set up this way(?). See my commits here. Couldn't say why, no one seems to be chiming in.

My fork seems to be working and passes all the tests, so I'm just going to use that for now. If no one has anything else to add, I'll submit a PR. As for static assets, at least those are cached by Cloudflare ¯\_(ツ)_/¯

@benmccann
Copy link
Member

maybe related to #7640 which @jrf0110 and @Rich-Harris had begun to look at a bit?

@The-Noah
Copy link
Contributor

Regardless of what is implemented, I think it would be extremely invaluable to be able to override - weather it's manually adding paths or removing paths that would be automatically added.

@Rich-Harris
Copy link
Member

The "new" way would be to deploy a [[path]].js file

To be clear, this is an authoring convenience only. Under the hood, your [[path]].js files get bundled into a _worker.js (unless the behaviour of Cloudflare Pages has changed since we last investigated this). SvelteKit is quite capable of emitting individual functions per route (it can do so for Vercel and Netlify with the split: true option), but there's no advantage to doing it on Cloudflare Pages.

I've redone my branch and modified how static pages are outputted. Interestingly, it seems like they were intentionally set up this way(?). See my commits here. Couldn't say why, no one seems to be chiming in.

See this conversation: #6530 (comment). The tl;dr is that someone might add their own _middleware.ts file, and @jrf0110 didn't want to prevent people from being able to run code in front of their assets. Personally I'd advocate for all assets being excluded, since that's how SvelteKit works on other platforms, but there's a maximum number of rules in _routes.json and it would be easy to exceed the limit if all static assets were included.

@colecrouter
Copy link
Author

colecrouter commented Nov 16, 2022

See this conversation: #6530 (comment).

Yes, that's what I was talking about in my earlier comments. Is there a way or us to track that "upstream auth" middleware, or do we just say "excluding prerendered routes is good enough for now".

–but there's a maximum number of rules in _routes.json

The (my) main concern is/was spending requests on just HTML assets (which are not cached by default on Cloudflare, along with worker responses). That being the case, calls to HTML files make up 80% of my current requests (again, until I bypass caching behaviour). In my own eyes, my revamped solution is "good enough".

Realistically, you'd have to bypass this behaviour anyway if your site has a lot of endpoint calls, but that's still a fairly big drop in worker calls (again, 80% for me, and I don't even have to add any hooks or anything for Cloudflare-CDN-Cache-Control headers onto everything). However, you're probably still gonna get plenty of bots/attackers/crawlers hitting / on your site, so caching headers don't help there. Implementing this change seems like a win-win-win to me.

*I can PR it later, I just want to make sure I'm not missing something obvious

@MedLeon
Copy link

MedLeon commented Nov 26, 2022

unless the behaviour of Cloudflare Pages has changed since we last investigated this

It didn´t. I was wrong. You were right. Sorry.
It really seems like _routes.json is the only way to bypass functions.

@LoaiDev
Copy link

LoaiDev commented Dec 21, 2022

what is the reason for creating a single _worker.js instead of a function for each endpoint? won't that mean cloudflare will serve the static assets on its own? or will github count a request to a file which didn't invoke a function (static) towards the usage anyways?

@colecrouter
Copy link
Author

colecrouter commented Dec 22, 2022

or will github count a request to a file which didn't invoke a function (static) towards the usage anyways?

Cloudflare charges per "function" aka per Worker invocation. Requests to static assets on the site are not billable.

what is the reason for creating a single _worker.js instead of a function for each endpoint?

Consistent behaviour across platforms. Cloudflare Pages might serve assets, match paths, match URLs, etc. differently from the way SvelteKit wants to. That being said, it's easier just to let SvelteKit handle all of it, even if it causes extra Worker invocations (billable).

For example, Cloudflare Pages lets you have a static 404.html page. If I wanted to do that with SvelteKit, I'd have to implement an +error.svelte file. The first approach is nice because it doesn't invoke a Worker but cannot be dynamic. The second approach is the opposite.

won't that mean cloudflare will serve the static assets on its own?

They get served the same way, and still get cached (luckily). The difference, as Rich mentioned above, is that every call to a static asset now invokes a Worker (unnecessarily, 99% of the time).

@benmccann
Copy link
Member

I believe this was just implemented in #8422

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants