Feat(cloudflare): configure prerender environment#15711
Conversation
🦋 Changeset detectedLatest commit: 6ed27df 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 |
packages/integrations/cloudflare/src/vite-plugin-dev-server-prerender-middleware.ts
Outdated
Show resolved
Hide resolved
|
@OliverSpeir please restore our PR template and fill it accordingly |
ematipico
left a comment
There was a problem hiding this comment.
Blocking because the testing part wasn't filled. Since this is a sensitive change, I want to make sure testing is done properly
There was a problem hiding this comment.
I'm not sure I understand why we're using this plugin. Can't we use the default Astro behaviour? It seems we're repeating code here, and we risk to diverge even more the dev behaviour from the "default" one
There was a problem hiding this comment.
Might have missed an obvious way to do this, how would you do it?
There was a problem hiding this comment.
If we don't load the CF plugin, Astro automatically uses its own dev server.
astro/packages/astro/src/vite-plugin-astro-server/plugin.ts
Lines 52 to 56 in 6acc91c
So the question is, what is this plugin actually doing? I read the documentation provided, and it seems it's meant for astro dev, correct?
There was a problem hiding this comment.
Yeah the scenario is we want pre-rendered pages to go through astro default dev server and on demand pages to go through workerd so that it aligns with the new option which uses node for prerendered pages instead of workerd
There was a problem hiding this comment.
another way would be if the cloudflare vite plugin accepted a shouldHandle function
could just return next() and then astros default dev server would handle it and we don't need this plugin at all
There was a problem hiding this comment.
We want to forward prerender pages to the astro environment in development, and prerenderer in build.
Yes in dev, in build we are actually just changing the prerenderer by not setting it to use the workerd one
We want to forward ssr pages to the ssr environment in development, and prerenderer in build.
on demand pages stay ssr for both dev and build
Currently all dev server requests go to ssr environment, and the cloudflare vite-plugin implements its own ssr which intercepts all requests indiscriminately via adding a vite middleware - so this PR creates a middleware that runs before the vite-plugin's and checks if the request is for a prerendered route and if it is it sends it to the astro environment which already exists but isnt used for dev server typically (no particular reason to use this environment its just that it already exists and has a runnable dev environment) and if it is not a prerendered route we let it go through to the ssr environment which is workerd
There was a problem hiding this comment.
@ematipico bump
created cloudflare/workers-sdk#12747 as well
There was a problem hiding this comment.
As a stop gap you could use configResolved and null out the dev plugin for cloudflare. https://github.com/cloudflare/workers-sdk/blob/main/packages/vite-plugin-cloudflare/src/plugins/dev.ts#L61
Pseudo-code:
const cloudflareDev = plugins.find(plugin => plugin.name === 'vite-plugin-cloudflare:dev');
cloudflareDev.configureServer = nullThere was a problem hiding this comment.
I do agree with James here though, that seems like a fairly clean way to do it: cloudflare/workers-sdk#12747
sarah11918
left a comment
There was a problem hiding this comment.
Great changeset @OliverSpeir ! 🎉
We'll just need a corresponding docs PR here in the Cloudflare adapter options section for prerenderEnvironment: https://v6.docs.astro.build/en/guides/integrations-guide/cloudflare/#options (You can use the headings for sessionKVBindingsName as a model for the API reference, since this one will need the <Since /> component too.)
Co-authored-by: Sarah Rainsberger <5098874+sarah11918@users.noreply.github.com>
2a5b9dd to
b328158
Compare
commit: |
ematipico
left a comment
There was a problem hiding this comment.
Unblocking. Let's merge after manual testing :)
|
|
I'm hitting an issue in my testing, looking into it. |
|
Ah, I didn't have the right version of Astro because there isn't a changeset. I'll add one. |
pnpm add https://pkg.pr.new/astro@71a927a
pnpm add https://pkg.pr.new/@astrojs/cloudflare@71a927a |
npm i https://pkg.pr.new/astro@76f3a50
npm i https://pkg.pr.new/@astrojs/cloudflare@76f3a50 |
Co-authored-by: Sarah Rainsberger <5098874+sarah11918@users.noreply.github.com>
This PR contains the following updates: | Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) | |---|---|---|---| | [astro](https://astro.build) ([source](https://github.com/withastro/astro/tree/HEAD/packages/astro)) | [`6.0.2` → `6.0.3`](https://renovatebot.com/diffs/npm/astro/6.0.2/6.0.3) |  |  | --- ### Release Notes <details> <summary>withastro/astro (astro)</summary> ### [`v6.0.3`](https://github.com/withastro/astro/blob/HEAD/packages/astro/CHANGELOG.md#603) [Compare Source](https://github.com/withastro/astro/compare/astro@6.0.2...astro@6.0.3) ##### Patch Changes - [#​15711](withastro/astro#15711) [`b2bd27b`](withastro/astro@b2bd27b) Thanks [@​OliverSpeir](https://github.com/OliverSpeir)! - Improves Astro core's dev environment handling for prerendered routes by ensuring route/CSS updates and prerender middleware behavior work correctly across both SSR and prerender environments. This enables integrations that use Astro's prerender dev environment (such as Cloudflare with `prerenderEnvironment: 'node'`) to get consistent route matching and HMR behavior during development. - [#​15852](withastro/astro#15852) [`1cdaf9f`](withastro/astro@1cdaf9f) Thanks [@​ematipico](https://github.com/ematipico)! - Fixes a regression where the the routes emitted by the `astro:build:done` hook didn't have the `distURL` array correctly populated. - [#​15765](withastro/astro#15765) [`ca76ff1`](withastro/astro@ca76ff1) Thanks [@​matthewp](https://github.com/matthewp)! - Hardens server island POST endpoint validation to use own-property checks for improved consistency </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My41OS41IiwidXBkYXRlZEluVmVyIjoiNDMuNTkuNSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==--> Co-authored-by: Renovate Bot <renovate@zarantonello.dev> Co-committed-by: Renovate Bot <renovate@zarantonello.dev>
Changes
Adds a
prerenderEnvironmentconfig option to cloudflare adapterCloses #15684
Adds
prerenderEnvironment: 'workerd' | 'node'to the adapter options (defaults to'workerd'). When set to'node', we skip the custom workerd-based prerenderer and the@cloudflare/vite-plugin prerenderworker config, falling back to Astro's built-in prerender environment instead.Makes
prerenderEnvironment: 'node'work duringastro devby adding a Vite plugin that intercepts requests for prerendered pages and renders them through theprerenderenvironment which is, currently, node instead of workerd.Testing
adds tests to start up dev server and verify it is using the correct environement
Docs
/cc @withastro/maintainers-docs for feedback!
withastro/docs#13389