adapter-cloudflare: allow disabling generation of fallback `404.html`

[ajgeiss0702]

Adds an option that will allow you to disable the automatic generation of 404.html if, for example, your site is fully dynamic and should not process any pages during build time.

The default option of auto will only generate the fallback page if there are any pre-rendered pages.

https://github.com/sveltejs/kit/issues/9386#issuecomment-1492500775

closes #9386

This PR also edits the docs to describe the new option.

I have tested this before posting, and the behavior of the option is as intended:

• generate404: undefined => generated only if there are any pre-rendered pages
• generate404: 'auto' => generated only if there are any pre-rendered pages
• generate404: true => generated
• generate404: false => not generated

If no pre-rendered page is generated, a simple 404.html is generated in /_app/404.html to prevent a resurgence of #9011 using src/error.html if it exists, otherwise the default error page.

---

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• [x] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• [x] This message body should clearly illustrate what problems it solves.
• [x] ~~Ideally, include a test that fails without this PR but passes with it.~~ N/A: adapter-cloudflare has no tests

Tests

• [x] Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• [x] If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: f2413f7bd6717c36b321bc41d446972bc2c6ce6e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/adapter-cloudflare	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[teemingc]

I'd go one step further and prevent generating a fallback if there are no prerendered pages in the cloudflare adapter. The main issue was invalid requests to /_app/** incorrectly returned 200 if you had a /index.html file. Without one, they should correctly return 404 although the page will just be a blank white one.

[Conduitry]

Would dealing with #9802 remove the need for this? It would be nice not to need another option.

[teemingc]

Would dealing with #9802 remove the need for this? It would be nice not to need another option.

I believe it would. The only reason for this option was that the fallback generation was breaking people's builds.

[ajgeiss0702]

Would dealing with #9802 remove the need for this? It would be nice not to need another option.

It would not. Completely removing the fallback page would cause any 404s to non-function routes to show the / page instead of a 404 page

In its current state, the option would only be needed if it breaks your build. It defaults to the same behavior as before.

I probably will get around to implementing @s3812497 ’s suggestion to automatically not generate it if there are no pre-rendered pages

[teemingc]

The proposed solution actually implements a static 404.html by default too. This would be without running hooks

[ajgeiss0702]

The proposed solution actually implements a static 404.html by default too. This would be without running hooks

It would implement it for the appDir, not the actual app. The fallback page would still be needed for the actual app to ensure that 404s in the app will have the correct behavior.

[teemingc]

It would implement it for the appDir, not the actual app. The fallback page would still be needed for the actual app to ensure that 404s in the app will have the correct behavior.

If the user has opted-in to use wildcard matching for static requests, perhaps fallback generation should be opt-in/ handled by them as well.

[ajgeiss0702]

It would implement it for the appDir, not the actual app. The fallback page would still be needed for the actual app to ensure that 404s in the app will have the correct behavior.

If the user has opted-in to use wildcard matching for static requests, perhaps fallback generation should be opt-in/ handled by them as well.

The simpler way would be, if unset, to only generate it if there are pre-rendered pages, as you suggested earlier

[ajgeiss0702]

The latest commit should address all previous issues with this.

• The default option for generate404 is now auto, which will only generate a fallback page if there are any pre-rendered pages
• If no fallback page is generated, then a simple 404 page is generated in the appDir from /src/errors.html (or the default if it doesn't exist) to prevent a resurgence of #9011

[Rich-Harris]

I've gone round in circles trying to understand this PR, #9386, #9011 and #9802, and here's where I'm at:

As far as I can tell, there's no real reason to ever generate an SPA-style fallback 404.html page, because there's always a _worker.js to render a 404 page on demand. (If you don't want a _worker.js, you should be using adapter-static.)

The only time we need a 404.html is to handle requests for non-existent assets that are covered by exclude in _routes.json — i.e. /_app/immutable/some-file-that-no-longer-exists.js. In that case, it doesn't need to be a fancy SPA-style fallback, it can literally just be a plaintext Not found response.

Am I missing anything?

[ajgeiss0702]

I've gone round in circles trying to understand this PR, #9386, #9011 and #9802, and here's where I'm at:

As far as I can tell, there's no real reason to ever generate an SPA-style fallback 404.html page, because there's always a _worker.js to render a 404 page on demand. (If you don't want a _worker.js, you should be using adapter-static.)

The only time we need a 404.html is to handle requests for non-existent assets that are covered by exclude in _routes.json — i.e. /_app/immutable/some-file-that-no-longer-exists.js. In that case, it doesn't need to be a fancy SPA-style fallback, it can literally just be a plaintext Not found response.

Am I missing anything?

Specifically my use case for it is that I generally exclude some routes that are usually checked by vulnerability scanners (to save on a few invocations). For example:

"/*/wp-includes",
"/*/wp-includes/*",
"/wp-includes",
"/wp-includes/*",
...etc

I would like for these to have the same 404s as the rest of my site.

---

Another use case I can think for it is prerendered pages with rest parameters.

For example, if you have a personal site that has some stuff that requires server logic, aswell as a static blog. You can exclude /blog/* to avoid risking going over the routes.json limit. If you just had a plaintext 404 page in 404.html, then users would get that when trying to go to a blog post that doesn't exist, or was removed.

Adding the option would allow devs to manually control whether the SPA is used or the static page is used, if there is some circumstance that was not thought of in this PR (like what happened in #9386 )

[Rich-Harris]

Yeah, I did consider the latter option, but thought it was probably overkill. But it would be easy enough to implement. If we were to do that I think we could ditch the 'is something prerendered or not?' logic because it seems a bit complicated and I'm not totally sure why it's relevant — it could just be an option like this:

import adapter from '@sveltejs/adapter-cloudflare';

export default {
  kit: {
    adapter: adapter({
      fallback: 'spa' // 'plaintext' (default) or 'spa'
    })
  }
};

Thoughts?

[Rich-Harris]

(Honestly, this would all be so much easier if Cloudflare just let you exclude all static assets, or did away with the arbitrary 100 route restriction, so that hacks like /blog/* weren't necessary in the first place. Weird design choice.)

[teemingc]

closed by https://github.com/sveltejs/kit/pull/11596 which now does not generate a fallback by default and provides the ability to do so through the adapter config.