withastro / adapters

Home for Astro's core maintained adapters
46 stars 25 forks source link

Is there actual astro:env support in Cloudflare Workers? #294

Closed janus-reith closed 1 week ago

janus-reith commented 1 week ago

Astro Info

Astro                    v4.11.0
Node                     v22.2.0
System                   macOS (arm64)
Package Manager          pnpm
Output                   server
Adapter                  @astrojs/node
Integrations             @astrojs/tailwind
                         @astrojs/alpinejs
                         @astrojs/react

(Not loaded locally):
@astrojs/cloudflare
@astrojs/vercel

Describe the Bug

Release 10.4.0 of the Cloudflare Adapter mentions support for astro:env. So far I was unable to get this working. Right after the build completes, the Deployment fails since the function throws about fields that e.g. I have defined as envField.string to not be of type text (probably missing).

I had issues with importing variables on Cloudflare before, import.meta.env.xyz didnt work. This ticket #8060 suggested a workaround of using context.locals.runtime.env.xyz - That works, but has some drawbacks:

Related: https://github.com/withastro/adapters/issues/208 https://github.com/withastro/astro/issues/8060

is there just something I'm missing, does anybody have a working example for cloudflare?

Regarding the stackblitz example: I did not link a reproducible example since i can't really test this within stackblitz I believe. I can try to create one later that can be deployed to CF that will fail.

What's the expected result?

I can pass environment variables to Cloudflare, preferably using the new astro:env feature. I had actually hoped for astro:env to solve the general issue with the way env has to be imported for Cloudflare.

Link to Minimal Reproducible Example

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

Participation

alexanderniebuhr commented 1 week ago

We have some bug with the support, we will revert it for v10, and v11 should have support for it. V11 should be released tomorrow.

janus-reith commented 1 week ago

@alexanderniebuhr Thanks for the quick reply, good to hear there's an upcoming fix! Is there any chance to test a prerelease version?

alexanderniebuhr commented 1 week ago

@janus-reith I saw your comment in the v11 PR. However all my tests work without an issue, can you provide a minimal reproduction, which shows your error.. I would like to see how you use the env variables, maybe there is a use-case we don't test for.

janus-reith commented 1 week ago

@alexanderniebuhr I'll create one now and try to narrow it down. Stackblitz seems to have some technical issues at the moment, in case that won't change until I'm done with the example I'l just provide a repo.

janus-reith commented 1 week ago

I noticed that the 10.4.2 adapter now warns in the console:

[config] The feature "astro:env getSecret" is not supported (used by @astrojs/cloudflare).
The adapter @astrojs/cloudflare doesn't support the feature envGetSecret. Your project won't be built. You should not use it.

Since there is no npm release for v11 yet, I'll include the adapter files in my repo again so I have something that I can actually verify to not work.

janus-reith commented 1 week ago

Ok this time I added the package code directly instead of a separate package in my monorepo and left out the original v10 adapter package. I realized I might've overlooked a detail here: https://github.com/withastro/adapters/blob/1c4145e1f9a27b2eee1f17f0689bf29345ba2ca6/packages/cloudflare/src/index.ts#L142

This probably causes by build to actually pick up the v10 entrypoint from before, sorry for the confusion.

janus-reith commented 1 week ago

I adjusted the entrypoints to use the local files and it's actually working now. Closing this as fixed in https://github.com/withastro/adapters/pull/290