Add warning when not excluding _app folder in Cloudflare Pages adapter

[LorenzoBloedow]

Ok so basically, I spent some time trying to debug a bunch of weird 404 errors that were being thrown when I deployed to Cloudflare Pages, after some time I realized it was because I was using a custom exclude array that didn't exclude the _app folder.

For some reason in my deployment I couldn't serve the _app folder from the server so it kept throwing a 404 error.
Initially, I thought of submitting a PR that would always exclude the _app folder, but there's probably a good reason why
SvelteKit allows for the developer to choose, so I thought it'd be nice to log a warning instead so anyone that gets in the same situation can hopefully not get stuck like me.

[changeset-bot]

🦋 Changeset detected

Latest commit: 8541f2c

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

[benmccann]

would probably need to check for <all> as well

[LorenzoBloedow]

would probably need to check for <all> as well

That wouldn't work because <all> gets mapped to <build>, <files> and <prerendered>.

This did make me realize that I made a mistake in checking after the mapping, and now the check happens before.
Checking after would mean that <build> would get converted to the app dir so there'd be no reason to check <build> in the first place.

And I was also checking for a hard-coded app directory which would break the check if the default app directory changes.

[benmccann]

lint is failing and will need to be fixed

Initially, I thought of submitting a PR that would always exclude the _app folder, but there's probably a good reason why
SvelteKit allows for the developer to choose

It looks like the PR implementing the feature originally did that, but @Rich-Harris suggested not to here: #9111 (comment). I'm not too well versed in this feature, but it sounds to me like it's required to exclude _app or else things break. If the only consequence of not excluding it is that your app breaks, I don't see the value in letting users do that. I think it can only result in a poor user experience. But I'm not that familiar with this feature though, so maybe I'm missing something. Thoughts @Rich-Harris?

[Conduitry]

Woah, what's this file?

[Conduitry]

Suggested change

	feat: Log warning when the \_app folder is not excluded
	feat: log warning when the `_app` folder is not excluded

[Rich-Harris]

okay, re-wrapping my head around the Cloudflare Pages _routes.json config. There's a few things going on here:

• leaving the app directory out of exclude should be fine — it should just result in an env.ASSETs.fetch here (which results in an unwanted function invocation, but nothing worse than that):

  kit/packages/adapter-cloudflare/src/worker.js

  Lines 36 to 38 in d93eb42

  	if (is_static_asset || prerendered.has(pathname)) {
  	res = await env.ASSETS.fetch(req);
  	} else if (location && prerendered.has(location)) {

  It's not doing that because we (incorrectly) only serve static files and prerendered pages. We can fix that, and when we do, the warning in this PR will be unnecessary
• In SvelteKit 2, we added a /_app/env.js module which is dynamically generated in certain cases. This must result in a function invocation. We forgot that we need to make sure we only treat certain things inside ${base}/_app (namely immutable and version.json) as static files

IOW we're basically talking about an entirely different chunk of work. I'll open a new PR.

[Rich-Harris]

closing in favour of #11593 — thanks