Skip to content

Fix Cloudflare preset not generating functioning applications#2116

Open
dario-piotrowicz wants to merge 2 commits intosolidjs:mainfrom
dario-piotrowicz:dario/fix-vite-plugin-nitro-2-cloudflare-presets
Open

Fix Cloudflare preset not generating functioning applications#2116
dario-piotrowicz wants to merge 2 commits intosolidjs:mainfrom
dario-piotrowicz:dario/fix-vite-plugin-nitro-2-cloudflare-presets

Conversation

@dario-piotrowicz
Copy link

@dario-piotrowicz dario-piotrowicz commented Mar 18, 2026

PR Checklist

Please check if your PR fulfills the following requirements:

What is the current behavior?

Setting up a Cloudflare preset in the plugin's configuration object results in builds not producing a functioning result (see #2115)

What is the new behavior?

A Cloudflare preset can be passed to the plugin's configuration object and builds do produce functioning applications.

Other information

To manually test the changes, create a solid start app via npm create solid, apply one of the cloudflare presets, run npm run build follows by the wrangler dev command that the build command presents in its output and see that the resulting application does not render. Try again but using this PRs pkg-pr-new and see that now it works as intended.

Note: I only tested the changes with cloudflare-module since, as far as I know, it's the generally preferred preset for Cloudflare deployments

@netlify
Copy link

netlify bot commented Mar 18, 2026

Deploy Preview for solid-start-landing-page ready!

Name Link
🔨 Latest commit ffdccd5
🔍 Latest deploy log https://app.netlify.com/projects/solid-start-landing-page/deploys/69bbf95e02f3540008d22e3b
😎 Deploy Preview https://deploy-preview-2116--solid-start-landing-page.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@changeset-bot
Copy link

changeset-bot bot commented Mar 18, 2026

🦋 Changeset detected

Latest commit: ffdccd5

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

@pkg-pr-new
Copy link

pkg-pr-new bot commented Mar 18, 2026

Open in StackBlitz

npm i https://pkg.pr.new/solidjs/solid-start/@solidjs/start@2116
npm i https://pkg.pr.new/solidjs/solid-start/@solidjs/vite-plugin-nitro-2@2116

commit: ffdccd5

Copy link

@themavik themavik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the changes — the implementation looks correct and addresses the issue well.

Copy link

@themavik themavik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What this PR does: Fixes Cloudflare preset builds by reconstructing full URLs from headers and using a custom virtual entry instead of fromWebHandler, which fails when Nitro strips URLs to path-only.

Done well: The getCloudflareVirtualEntryContent correctly rebuilds the URL from host, x-forwarded-proto, and path. The getHeader helper handles both Headers-like and plain object header formats. Setting noExternals for Cloudflare presets avoids conditional export resolution issues. Good separation in cloudflare.ts.

Edge case: The path construction uses event.path || event.req.url - consider x-forwarded-host for proxied setups.

@brenelz
Copy link
Contributor

brenelz commented Mar 19, 2026

Is it possible to add tests for this? Not sure how feasible with it being so tied to cloudflare

Copy link
Contributor

@brenelz brenelz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im told this will break some other adapters

@atilafassina
Copy link
Member

atilafassina commented Mar 20, 2026

Im told this will break some other adapters

can you please elaborate, @brenelz / @huseeiin ?
changes seem scoped to Cloudflare presets to me

@huseeiin
Copy link
Contributor

can you please elaborate, @brenelz / @huseeiin ? changes seem scoped to Cloudflare presets to me

i said it will break netlify i was wrong it will break #2056 because noExternals: isCloudflarePreset(preset) will make noExternals only true if using cloudflare

@dario-piotrowicz
Copy link
Author

Is it possible to add tests for this? Not sure how feasible with it being so tied to cloudflare

I asked the same in the PR's description 🙂

I think that the logic of the plugin is not currently being tested and having full e2e/integration tests for this could be a bit involved 🤔

But If we wanted to, I think we could somewhat easily have some unit tests to ensure that the various functions return the expected values

Would you like me to give that a go and actually set up tests for the package here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug?]: nitroV2Plugin doesn't generate a valid output with Cloudflare presets

5 participants