withastro / adapters

Home for Astro's core maintained adapters
64 stars 33 forks source link

`env.ASSETS.fetch` API doesn't exist in CF workers #273

Closed NOBLES5E closed 4 months ago

NOBLES5E commented 4 months ago

Astro Info

Astro                    v4.9.2
Node                     v18.20.3
System                   Linux (x64)
Package Manager          pnpm
Output                   server
Adapter                  @astrojs/cloudflare
Integrations             @nevix/custom_config
                         @astrojs/react
                         @astrojs/tailwind

Describe the Bug

When deploying an Astro website using Cloudflare Workers instead of Cloudflare Pages to avoid certain limitations, the env.ASSETS.fetch function is not defined in the Workers environment. This causes issues after the project gets deployed (for example, when visiting a page that is supposed to be redirected to 404 page).

It's worth noting that I'm not entirely sure if Cloudflare Workers are intended to be officially supported by Astro. Currently, Cloudflare Pages have major limitations, such as requiring control of the entire domain (https://community.cloudflare.com/t/plans-to-provide-a-solution-like-workers-routes-for-cf-pages-apps/614706), which forces us to explore alternative deployment options like Cloudflare Workers.

To support the Cloudflare Workers environment, it would be beneficial to add a fallback mechanism when env.ASSETS.fetch is not available. This fallback could use an alternative method to fetch assets or provide a suitable default behavior.

Proposed Solution:

Modify the code in packages/cloudflare/src/entrypoints/server.ts to include a fallback mechanism when env.ASSETS.fetch is not available. This could involve checking for the existence of env.ASSETS.fetch and using an alternative method to fetch assets or providing a default behavior when it is undefined.

Link to the relevant code:

https://github.com/withastro/adapters/blob/be0b27de38418330898205d506fad87b7da563be/packages/cloudflare/src/entrypoints/server.ts#L35

What's the expected result?

The deployment should succeed without any errors related to env.ASSETS.fetch being undefined. The fallback mechanism should handle the absence of env.ASSETS.fetch gracefully.

Link to Minimal Reproducible Example

https://stackblitz.com/github/withastro/astro/tree/latest/examples/basics

Participation

alexanderniebuhr commented 4 months ago

@NOBLES5E Thank you for opening the issue. And sorry for the inconvience.

The adapter officially doesn't support Cloudflare Workers anymore. Additionally I remember reading that env.ASSETS should be available in Cloudflare Workers in the future, I'm not sure when that will happen though. I would normally close this Issue, because we would not work on this, but given that you already put some thought into it and have an idea, I would accept a PR if the change doesn't interfere with Cloudflare Pages, therefore not closing this issue, but just labeling as wontfix.

Other options could be to implement this in userland, patch the adapter or create a dedicated community one for Cloudflare workers

NOBLES5E commented 4 months ago

@alexanderniebuhr Thank you for the response and clarification. I appreciate your willingness to consider a PR.

After further consideration, I think it's best to close this issue for now. We have decided to go with the pnpm patch route since we can skip the env.ASSETS.fetch lines as we host static assets separately. Implementing a general solution for Cloudflare Workers would likely require uploading assets to workers as well, which we don't have a good solution for at the moment.

If we come up with a more comprehensive approach that could benefit the wider community, we'll consider opening a new issue or submitting a PR in the future. Thank you for your time and attention to this matter. Feel free to close the issue whenever convenient.