Repository navigation
Conversation
|
Install the latest version of pnpm add https://pkg.svelte.dev/@sveltejs/kit/c/d218d583b3d7483b33ed22b8615eacaedf010341Open in |
🦋 Changeset detectedLatest commit: d218d58 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
…' into claude/cloudflare-workers-build-output # Conflicts: # documentation/docs/25-build-and-deploy/60-adapter-cloudflare.md # packages/adapter-cloudflare/index.spec.js
| // we need the `assets.directory` key so that the static assets are deployed | ||
| if ((wrangler_config.main || wrangler_config.assets) && !wrangler_config.assets?.directory) { | ||
| throw new Error( | ||
| `You must specify the \`assets.directory\` key in ${config_path}. Consult https://developers.cloudflare.com/workers/static-assets/binding/#directory` | ||
| ); | ||
| } | ||
|
|
||
| // we need the `assets.binding` key so that the Worker can access the static assets | ||
| if (wrangler_config.main && !wrangler_config.assets?.binding) { | ||
| throw new Error( | ||
| `You must specify the \`assets.binding\` key in ${config_path} before deploying your Worker. Consult https://developers.cloudflare.com/workers/static-assets/binding/#binding` | ||
| ); | ||
| } | ||
|
|
||
| // the user might have forgot the `main` key or should remove the `assets.binding` | ||
| // key to deploy static assets without a Worker | ||
| if (!wrangler_config.main && wrangler_config.assets?.binding) { | ||
| throw new Error( | ||
| `You must specify the \`main\` key in ${config_path} if you want to deploy a Worker alongside your static assets. Otherwise, remove the \`assets.binding\` key if you only want to deploy static assets.` | ||
| ); | ||
| } |
There was a problem hiding this comment.
adapter generates overrides for these so the user never needs to specify them anymore
| "dev": "vite dev", | ||
| "build": "vite build", | ||
| "preview": "wrangler dev dist/index.js --config config/wrangler.jsonc", | ||
| "preview": "wrangler dev", |
There was a problem hiding this comment.
we generate a deployment config that wrangler automatically picks up, so users don't need to specify the build output path or user wrangler config path anymore
| // a Vite root that differs from the working directory | ||
| const root = fs.mkdtempSync(path.join(os.tmpdir(), 'adapter-cloudflare-')); | ||
|
|
||
| test('selects Wrangler configuration and named environments from Vite env files', async () => { |
There was a problem hiding this comment.
…adapter config path now resolves for the Cloudflare build plugins but fails in the development platform proxy, preventing the dev server from starting. This commit fixes the issue reported at packages/adapter-cloudflare/config.js:36 This change makes `options.config` relative to Vite's root, but `virtual_workers_module` still passes that same relative string directly to Wrangler's `getPlatformProxy` and `unstable_readConfig`, which resolve it against the working directory. Reproduced from the adapter directory with Vite root `test/apps/workers` and adapter config `config/wrangler.jsonc`: configuration resolves, but dev-server setup fails with `Could not read file: config/wrangler.jsonc`. The suggested patch resolves the proxy's config path against the resolved Vite root before setup and watching. With the patch, the same dev server starts and its platform binding matches the selected Wrangler config. Adapter type checks passed. Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com> Co-authored-by: teemingc <chewteeming01@gmail.com>
…ev` loses them because the generated `.dev.vars` stays inside `worker/server` while the deployment config moves to `worker`. This commit fixes the issue reported at packages/adapter-cloudflare/index.js:53-55 Cloudflare's output-config plugin emits a `.dev.vars` alongside the original server build. `writeServer` copies it into `worker/server`, but the adapter puts the final `wrangler.json` one directory above it. Wrangler loads local secrets relative to the selected config's directory, so the new `wrangler dev` preview workflow no longer sees these bindings (or uses the deployed `vars` value instead of the local override). Reproduced by building the Workers app with a local `FOO` override: Wrangler's installed `unstable_getVarsForDev` returns that override for the source config but no bindings for the final deployment config. Moving the emitted file alongside the final config restores the override. Rebuilt the app and verified the binding using the same Wrangler API; adapter type checks passed. Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com> Co-authored-by: teemingc <chewteeming01@gmail.com>
…e forwarded Cloudflare `configResolved` hook runs while the plugins that initialize its context are disabled for preview. This commit fixes the issue reported at packages/adapter-cloudflare/config.js:115-130 Reproduced with the Workers test app: `vite preview` exits with `AssertionError: Expected resolvedPluginConfig to be defined` before starting the server. The new wrapper has no `apply` guard, unlike the initialization and environment plugins, so its forwarded hook accesses an uninitialized Cloudflare context. The suggested patch gives this wrapper the same dev/build guard. Validated by resolving Vite configuration with `isPreview: true`, which succeeds with the patch. Adapter unit tests and type checks also passed before this change. Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com> Co-authored-by: teemingc <chewteeming01@gmail.com>
| @@ -0,0 +1 @@ | |||
| export const variables = { FOO: {} }; | |||
There was a problem hiding this comment.
Same issue as last PR with FOO here. Currently we don't pick up variables defined in wrangler.jsonc, and I don't think we should.
There was a problem hiding this comment.
Mm should we not? I think I overlooked this. There's an existing issue about this where it's a hassle to manually sync the vars in wrangler.jsonc and in .env. Maybe we should load them during dev, build, and preview? From the wrangler file property description it says: "A map of environment variables to set when deploying your Worker." so it seems like they're expected to be present wherever.
There was a problem hiding this comment.
Hmm I guess we should if there are issues open about it. We can probably do that pretty easily as a vite plugin, right? I don't know how hard it is to augment vite's env variables.
closes #2963
closes #13300
This PR uses the Cloudflare Vite plugin to create a suitable deployment artefact for Cloudflare Workers. Therefore, we get all the goodies such as: