feat(@astrojs/cloudfalre): add cloudflare envs to Astro.locals

[alexanderniebuhr]

Changes

• Add the Cloudflare Runtime Variables as Astro.locals.runtime
• Add deprecation comment in code, to make sure we remove getRuntime() future release
• Add migration info in changeset
• Update Docs
• closes [astrojs/cloudflare] use Astro.locals for env variable #7538

Testing

• manually tested with Cloudflare Pages Deployments.
• duplicated integration test, which was used for getRuntime

Docs

/cc @withastro/maintainers-docs for feedback!

Notes:

• It does not export a type yet. Users have to type the locals themself.

[changeset-bot]

🦋 Changeset detected

Latest commit: 45a52b2

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

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

[matthewp]

Hey @alexanderniebuhr 👋 this is looking good, but is still in draft. Are you ready for people to review it?

[alexanderniebuhr]

Are you ready for people to review it?

@matthewp soon! Need to fix some types and double check a test! And it still needs docs! Will ping you here & in Discord :)

[alexanderniebuhr]

@matthewp I'm ready to get initial reviews :)

[ematipico]

It would be great if we can add some tests, where we have a middleware.

Inside the middleware, should do something like:

• context.locals.user.name = "Nick" -> no errors, it should work
• context.locals.runtime = {} -> runtime error, can't override
• context.locals = {}, allowed.. no runtime errors but log a warning to the user that wiping out locals is unsafe

[sarah11918]

Docs approves the changeset and README! 🥳

[lilnasy]

Would Astro.locals in the frontmatter have identical functionality to context.locals in the middleware?

For context, trying to decide on a different API for #7975, and this PR seems related.

[alexanderniebuhr]

Would Astro.locals in the frontmatter have identical functionality to context.locals in the middleware?

AFAIK, no. This just passes the cf runtime to locals using the app.render() function.

[ematipico]

Almost there! I think we're good. Just one more thing: could you add a TODO comment to the getRuntime function as a reminder to remove it in the next major release of the library??

[lilnasy]

I would like there to be a convention for adapters introducing runtime controls through locals.

For #7975, I went with Astro.locals.netlify.setBuildersTtl() because I wanted it to be immediately clear that you might need to revisit it if you switch providers. But, I am not opposed to Astro.locals.runtime.setBuildersTtl().

What I consider important is that there is consistency.

[alexanderniebuhr]

I would like there to be a convention for adapters introducing runtime controls through locals.

I am also fine with both. However I think this needs some RFC to align across everything. I really want this PR to get merged soon, so not sure if we can wait for the RFC. Any opinions cc @ematipico @matthewp

• Astro.locals.<PROVIDER>.<XYZ>
• Astro.locals.runtime.<XYZ>

[matthewp]

lgtm! No opinion on namespacing from me, integrations can essentially do what they want here, I don't think an RFC is valid for this sort of thing. Adapters can coordinate if they'd like / think it makes things easier. I'm not sure if it does though, but that's my opinion.

[natemoo-re]

Looks great!

One request from me: could we mark the getRuntime export as /** @deprecated */ so that users get this feedback in their IDE?

[lilnasy]

In that case, I'll go with locals.runtime because I'd only have to change one line.