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: only including _app content in _routes.json #7640

Closed
userquin opened this issue Nov 14, 2022 · 15 comments · Fixed by #8422
Closed

adapter-cloudflare: only including _app content in _routes.json #7640

userquin opened this issue Nov 14, 2022 · 15 comments · Fixed by #8422
Labels
bug Something isn't working pkg:adapter-cloudflare
Milestone

Comments

@userquin
Copy link
Contributor

userquin commented Nov 14, 2022

Describe the bug

The adapter should include any assets inside the client output folder instead _app only (kit.appDir).

Latest changes in kit allow plugins to emit/generate assets in the client folder: @vite-pwa/sveltekit, sitemap...

The problem is here: https://github.com/sveltejs/kit/blob/master/packages/adapter-cloudflare/index.js#L42 (should include the client folder)

Reproduction

no, sorry, just reported from Discord here: https://discord.com/channels/937808017016119440/937973377883336704/1041084393239891988

https://github.com/hazre/neosvr-mod-manager/tree/fix/pwa

Logs

No response

System Info

NA

Severity

blocking all usage of SvelteKit

Additional Information

The app is using the new @vite-pwa/sveltekit plugin to add the PWA, cloudflare returns 404 for the service worker and webmanifest assets.

@benmccann benmccann added the bug Something isn't working label Nov 14, 2022
@benmccann benmccann added this to the 1.0 milestone Nov 14, 2022
@hazre
Copy link

hazre commented Nov 14, 2022

Here's a list of the cloudflare folder. Assets are definitely there and they work fine locally with pnpm run preview but when I deploy it to cloudflare pages, I get a 404s, which is weird.

https://gist.github.com/hazre/1ebf72e8f43f0252822076aa6c33b3c6

@benmccann
Copy link
Member

@jrf0110 tagging you in case you or anyone else from Cloudflare might be interested in giving this one a look. We no longer have any Svelte maintainers employed at Cloudflare and I don't have a Cloudflare account so couldn't be very confident in any attempt I made

@tedsteen
Copy link

When deploying to Cloudflare with kit 1.0.0-next.544 I don't get any functions deployed. do I understand correctly that this issue related to that?

@jrf0110
Copy link
Contributor

jrf0110 commented Nov 15, 2022

@benmccann I'll get some 👀 on this today! Thanks for the ping

@Rich-Harris
Copy link
Member

@tedsteen that sounds like a separate issue

@Rich-Harris
Copy link
Member

started looking at this, wrote up notes in #7667

@jrf0110
Copy link
Contributor

jrf0110 commented Nov 15, 2022

Just to clarify, the _routes.json file is an optimization. Missing entries should not cause any assets to 404

@userquin
Copy link
Contributor Author

userquin commented Nov 15, 2022

Just to clarify, the _routes.json file is an optimization. Missing entries should not cause any assets to 404

Yeah, sorry for that, maybe we can change the title and the content: I just check the code, I never use CloudFlare, I assume the routes file being used for routing.

@userquin
Copy link
Contributor Author

Generated _routes.json sent by @hazre after I created this issue:

{
  "version": 1,
  "description": "Generated by @sveltejs/adapter-cloudflare",
  "include": [
    "/*"
  ],
  "exclude": [
    "/_app/immutable/*",
    "/favicon-16x16.png",
    "/favicon-32x32.png",
    "/favicon.ico"
  ]
}

@jrf0110
Copy link
Contributor

jrf0110 commented Nov 15, 2022

Is there a bug to report here? Is the listed severity correct?

blocking all usage of SvelteKit

@Neicul123
Copy link

Am I correct in assuming this would also fix the version.json calls from counting towards the quota on CF? Because I seem to be running into the problem where if one (or a few) users leave open their tab, it will do thousands of function requests, quickly ramping up costs.

@Rich-Harris
Copy link
Member

Yes. Alternative this line...

`/${app_dir}/immutable/*`,
...would need to lose the immutable part.

@jrf0110 is there any likelihood that we could just exclude all static assets (perhaps behind an option in the adapter), without adding them individually to the exclude array (and exceeding the 100 limit, in many cases)?

@0xMihir
Copy link

0xMihir commented Nov 24, 2022

I'm unsure if this is related, but any generated items are not added to the manifest.

I'm running into a similar issue to the one mentioned here (xiphux/svimg#16 (comment)). This package outputs transformed images to a subfolder inside the cloudflare directory, like so:
image

All of the images in the generated folder return 404 when trying to access them, but the two untransformed images in the images/ folder are accessible.

However, none of the generated images are in the manifest, which seems to be the reason why they aren't served.
image

If I made the build run twice, it will output these files to the manifest, which allows Cloudflare to recognize and serve the files. I feel the solution to this could be adding generated files to the manifest after running other preprocess steps.

@ajgeiss0702
Copy link
Contributor

ajgeiss0702 commented Nov 29, 2022

Is there a bug to report here? Is the listed severity correct?

blocking all usage of SvelteKit

Only a slight exaggeration. Currently, almost any request that goes through would count as a function invocation. My site is mostly static except for a few (4) endpoints that must have server-side code. According to Cloudflare analytics, my site gets 1-1.5 million (uncached) requests on an average day (and is growing). The endpoints only get ~10,000 requests per day.

This issue is the difference between my side-project being free and it being yet another subscription I have to pay for (seemingly unnecessarily, since the sections that actually need functions take up <1% of the total requests).

My site is growing, so I would probably have to pay for it at some point, but when I do, it would be much better if I only had to pay for the function requests I actually use, instead of the ones just serving static content.

Currently, I'm using svelte-multi-adapter to use both the static adapter and the nodejs adapter. I'm then deploying the static site to cloudflare pages, (for most of the content on the website) then the nodejs generated server to a vps I have to host somewhere else. Obviously, this setup is not ideal.

Rich-Harris added a commit that referenced this issue Jan 19, 2023
* include as many static assets as possible in exclude list - closes #7640

* temporarily add trailing slash

* Revert "temporarily add trailing slash"

This reverts commit f3ff35f.

* add a warning

* fix typo

* fix string

* Update .changeset/giant-penguins-act.md

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>

* fix

Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pkg:adapter-cloudflare
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants