feat: add Cloudflare deployment adapter - #191
Conversation
Add @marko/run-adapter-cloudflare for deploying Marko Run apps to Cloudflare Workers (default) or Cloudflare Pages. - Builds for the webworker SSR target with workerd export conditions - Workers mode emits a bundled `_worker.js` plus a generated wrangler.json (skipped when a project-level Wrangler config exists) - Pages mode emits an advanced-mode `_worker.js` and a `_routes.json` that excludes static assets from the function - Exposes env/ctx/cf to route handlers via CloudflarePlatformInfo - Previews via the Wrangler CLI (`wrangler dev` / `wrangler pages dev`)
Add dev test fixtures for both Cloudflare adapter modes, mirroring the existing Netlify adapter fixtures. Preview is skipped because it requires the Wrangler CLI (unavailable in CI), matching the Netlify edge fixture. Verified the build output for each mode: - workers: dist/_worker.js (default fetch export with ASSETS fallback) and a generated dist/wrangler.json pointing at the worker and public assets - pages: dist/public/_worker.js plus dist/public/_routes.json excluding the static assets dir
🦋 Changeset detectedLatest commit: 34198b7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
Add @marko/run-adapter-vercel for deploying Marko Run apps to Vercel via
the Build Output API (v3), emitting a `.vercel/output` directory.
- Node.js Serverless Functions by default; `{ edge: true }` targets the
Edge runtime (webworker SSR target with the edge-light export condition)
- Node mode wraps the fetch handler with createMiddleware as the function
handler; static assets are served by Vercel's filesystem layer
- Generates functions/index.func with the appropriate .vc-config.json,
copies static assets to output/static, and writes a routing config.json
- Exposes platform info per runtime via VercelEdgePlatformInfo /
VercelNodePlatformInfo
- Adds run-package test fixtures for both modes (preview skipped; it needs
the Vercel CLI)
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (26)
WalkthroughThis pull request adds new 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
packages/run/src/__tests__/fixtures/cloudflare-adapter-workers/package.json (1)
1-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider marking fixture package as private.
Test fixture packages are typically not meant to be published. Adding
"private": trueavoids accidental publishing if this monorepo ever does a wildcard publish.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/run/src/__tests__/fixtures/cloudflare-adapter-workers/package.json` around lines 1 - 5, Mark the fixture package as private in the cloudflare-adapter-workers package manifest so it cannot be published accidentally. Update the package.json for this test fixture by adding the private flag alongside the existing name, version, and scripts fields.packages/adapters/cloudflare/src/index.ts (1)
206-227: 🚀 Performance & Scalability | 🔵 Trivial
parseWranglerArgssilently drops space-separated flag values.The
devFlagspatterns require an=(e.g.ip=.+), so a pass-through arg like--ip 127.0.0.1(two array entries) would have--ipfiltered out (no match) and127.0.0.1filtered out too (matches nothing), silently dropping user-supplied CLI options. Worth documenting that only--flag=valueform is forwarded, or supporting both forms.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/adapters/cloudflare/src/index.ts` around lines 206 - 227, The issue is that parseWranglerArgs only forwards flags matched by devFlags, so space-separated options like --ip 127.0.0.1 are dropped. Update the parsing in parseWranglerArgs/devFlags to either support both --flag=value and --flag value forms, or make the behavior explicit by only forwarding documented equals-style args; ensure the handling around the flag list in packages/adapters/cloudflare/src/index.ts preserves user-supplied CLI options correctly.packages/adapters/cloudflare/tsconfig.json (1)
4-4: 📐 Maintainability & Code Quality | 🔵 TrivialConfirm
default-entry.tsexclusion fromtsc -bis intentional.Excluding
src/default-entry.tsmeans it's never type-checked bytsc -b(only bundled via esbuild inscripts/build.ts, which doesn't type-check). If this is to avoid ambient-type conflicts between Cloudflare Workers types and the base tsconfig's lib/types, that's reasonable — but worth a comment explaining why, since otherwise type regressions in this file go unnoticed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/adapters/cloudflare/tsconfig.json` at line 4, Confirm that excluding default-entry.ts from the Cloudflare adapter tsconfig is intentional by updating the tsconfig.exclude entry and adding a brief comment near the relevant config or build entrypoint (such as default-entry.ts or the build script) explaining that tsc -b intentionally skips it because it is bundled separately and has ambient type conflicts with the base Cloudflare setup. Keep the exclusion if intended, but make the rationale explicit so maintainers know why default-entry.ts is not type-checked by tsc -b.packages/adapters/cloudflare/scripts/build.ts (1)
45-50: 🚀 Performance & Scalability | 🔵 Trivial
platform: "node"applied to the Workers-runtimedefault-entry.tsbuild.The
default-entry.tsbuild (entry executed insideworkerd, not Node) inheritsplatform: "node"fromopts. This affects esbuild's default conditions/built-in handling and is semantically mismatched for code targeting the Workers runtime. Ifdefault-entry.tshas no Node built-in dependencies this is likely harmless today, but considerplatform: "neutral"or"browser"for this entry to avoid surprises if Node-specific resolution ever leaks in.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/adapters/cloudflare/scripts/build.ts` around lines 45 - 50, The Workers-runtime build for default-entry.ts is inheriting the Node platform from opts, which is a mismatch for code executed in workerd. Update the build call in build.ts that targets src/default-entry.ts to override platform to a Workers-appropriate value such as neutral or browser instead of carrying through the Node setting, while keeping the rest of the esbuild options unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/adapters/cloudflare/package.json`:
- Around line 16-22: The Cloudflare adapter package is publishing entrypoints
that point to source files that are not included in the tarball. Update the
package metadata in package.json so exports["."] and types resolve to the built
output under dist rather than src/index.ts, or alternatively expand files to
include the source tree; make the published paths consistent with what gets
shipped.
In `@packages/adapters/cloudflare/scripts/importMetaURL.js`:
- Around line 1-2: `__importMetaURL` is currently exporting a URL object from
`importMetaURL.js`, which causes `path.dirname()` to fail when the Cloudflare
CJS bundle loads it. Update the `__importMetaURL` export to be a string URL
instead of the `pathToFileURL(__filename)` object, keeping the existing
`pathToFileURL` usage but returning its `.href` so consumers like the
module-load path resolution logic receive the expected string value.
In `@packages/adapters/cloudflare/src/index.ts`:
- Around line 68-105: The workers-mode preview path is hardcoded to use a
generated wrangler.json, which breaks when a root Wrangler config already exists
and buildEnd does not create that file. Update startPreview to choose the config
source based on the same hasRootWranglerConfig/buildEnd logic, so wrangler dev
is pointed at an existing config or skips --config when the project’s own root
config should be used. Keep the behavior aligned between buildEnd and
startPreview by using the shared mode/hasRootWranglerConfig branching.
- Around line 87-91: The Wrangler process launch in spawn should not use shell
mode. Update the spawn call in the proc creation path to remove shell: true so
Wrangler receives the publicDir, --config, and filtered args directly as argv
entries; keep the existing cwd and env handling in place.
---
Nitpick comments:
In `@packages/adapters/cloudflare/scripts/build.ts`:
- Around line 45-50: The Workers-runtime build for default-entry.ts is
inheriting the Node platform from opts, which is a mismatch for code executed in
workerd. Update the build call in build.ts that targets src/default-entry.ts to
override platform to a Workers-appropriate value such as neutral or browser
instead of carrying through the Node setting, while keeping the rest of the
esbuild options unchanged.
In `@packages/adapters/cloudflare/src/index.ts`:
- Around line 206-227: The issue is that parseWranglerArgs only forwards flags
matched by devFlags, so space-separated options like --ip 127.0.0.1 are dropped.
Update the parsing in parseWranglerArgs/devFlags to either support both
--flag=value and --flag value forms, or make the behavior explicit by only
forwarding documented equals-style args; ensure the handling around the flag
list in packages/adapters/cloudflare/src/index.ts preserves user-supplied CLI
options correctly.
In `@packages/adapters/cloudflare/tsconfig.json`:
- Line 4: Confirm that excluding default-entry.ts from the Cloudflare adapter
tsconfig is intentional by updating the tsconfig.exclude entry and adding a
brief comment near the relevant config or build entrypoint (such as
default-entry.ts or the build script) explaining that tsc -b intentionally skips
it because it is bundled separately and has ambient type conflicts with the base
Cloudflare setup. Keep the exclusion if intended, but make the rationale
explicit so maintainers know why default-entry.ts is not type-checked by tsc -b.
In `@packages/run/src/__tests__/fixtures/cloudflare-adapter-workers/package.json`:
- Around line 1-5: Mark the fixture package as private in the
cloudflare-adapter-workers package manifest so it cannot be published
accidentally. Update the package.json for this test fixture by adding the
private flag alongside the existing name, version, and scripts fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dde9d894-4ce4-4c24-b3a3-04aaf81849a4
⛔ Files ignored due to path filters (3)
package-lock.jsonis excluded by!**/package-lock.jsonand included by**packages/run/src/__tests__/fixtures/cloudflare-adapter-pages/__snapshots__/dev.expected.mdis excluded by!**/__snapshots__/**and included by**packages/run/src/__tests__/fixtures/cloudflare-adapter-workers/__snapshots__/dev.expected.mdis excluded by!**/__snapshots__/**and included by**
📒 Files selected for processing (26)
.changeset/cloudflare-adapter.mdcspell.jsonpackages/adapters/cloudflare/README.mdpackages/adapters/cloudflare/package.jsonpackages/adapters/cloudflare/scripts/build.tspackages/adapters/cloudflare/scripts/importMetaURL.jspackages/adapters/cloudflare/src/default-entry.tspackages/adapters/cloudflare/src/index.tspackages/adapters/cloudflare/src/types.tspackages/adapters/cloudflare/tsconfig.jsonpackages/run/src/__tests__/fixtures/cloudflare-adapter-pages/.gitignorepackages/run/src/__tests__/fixtures/cloudflare-adapter-pages/.marko-run/routes.d.tspackages/run/src/__tests__/fixtures/cloudflare-adapter-pages/package.jsonpackages/run/src/__tests__/fixtures/cloudflare-adapter-pages/src/components/counter.markopackages/run/src/__tests__/fixtures/cloudflare-adapter-pages/src/routes/+page.markopackages/run/src/__tests__/fixtures/cloudflare-adapter-pages/test.config.tspackages/run/src/__tests__/fixtures/cloudflare-adapter-pages/tsconfig.jsonpackages/run/src/__tests__/fixtures/cloudflare-adapter-pages/vite.config.tspackages/run/src/__tests__/fixtures/cloudflare-adapter-workers/.gitignorepackages/run/src/__tests__/fixtures/cloudflare-adapter-workers/.marko-run/routes.d.tspackages/run/src/__tests__/fixtures/cloudflare-adapter-workers/package.jsonpackages/run/src/__tests__/fixtures/cloudflare-adapter-workers/src/components/counter.markopackages/run/src/__tests__/fixtures/cloudflare-adapter-workers/src/routes/+page.markopackages/run/src/__tests__/fixtures/cloudflare-adapter-workers/test.config.tspackages/run/src/__tests__/fixtures/cloudflare-adapter-workers/tsconfig.jsonpackages/run/src/__tests__/fixtures/cloudflare-adapter-workers/vite.config.ts
| "exports": { | ||
| ".": "./src/index.ts" | ||
| }, | ||
| "types": "./src/index.ts", | ||
| "files": [ | ||
| "dist" | ||
| ], |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if sibling adapter packages follow the same src-pointing exports pattern,
# and look for any publish/prepack scripts that rewrite package.json fields.
fd package.json packages/adapters --exec cat {} \;
fd -t f '(prepack|publish)' --extension js --extension ts -1
rg -n '"exports"|"types"|"files"' -g 'packages/adapters/*/package.json'Repository: marko-js/run
Length of output: 3679
🏁 Script executed:
#!/bin/bash
rg -n '"prepack"|"publish"|"changeset"|"version-packages"|publishConfig|postpack|prepare' -g 'package.json' -g '.changeset/**' -g '.github/**' -g 'scripts/**' .Repository: marko-js/run
Length of output: 192
Published package points to files that are not shipped. exports["."] and types resolve to ./src/index.ts, but files only includes dist, so consumers will hit missing-file errors from the published tarball. Update these fields to dist (or include the source files in the package).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/adapters/cloudflare/package.json` around lines 16 - 22, The
Cloudflare adapter package is publishing entrypoints that point to source files
that are not included in the tarball. Update the package metadata in
package.json so exports["."] and types resolve to the built output under dist
rather than src/index.ts, or alternatively expand files to include the source
tree; make the published paths consistent with what gets shipped.
| import { pathToFileURL } from "url"; | ||
| export const __importMetaURL = pathToFileURL(__filename); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Node.js url.pathToFileURL return type URL object vs string
💡 Result:
The Node.js url.pathToFileURL() function returns a WHATWG URL object, not a string [1]. It does not return a string; the object returned is an instance of the global URL class [1]. This function is designed to convert a file system path into a properly formatted and encoded file:// URL, ensuring that the path is resolved absolutely and that any URL control characters are correctly handled [2][3]. If you require a string representation of the resulting URL, you must explicitly call the .toString() method or access the .href property of the returned URL object [2].
Citations:
- 1: https://beta.docs.nodejs.org/url.html
- 2: https://nodejs.org/dist/latest/docs/api/url.html
- 3: https://nodejs.org/api/url.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- files ---\n'
git ls-files 'packages/adapters/cloudflare/scripts/importMetaURL.js' 'packages/adapters/cloudflare/scripts/build.ts' 'packages/adapters/cloudflare/src/index.ts'
printf '\n--- importMetaURL.js ---\n'
cat -n packages/adapters/cloudflare/scripts/importMetaURL.js
printf '\n--- build.ts matches ---\n'
rg -n 'import\.meta\.url|__importMetaURL|define' packages/adapters/cloudflare/scripts/build.ts packages/adapters/cloudflare/src/index.ts
printf '\n--- index.ts relevant lines ---\n'
sed -n '1,120p' packages/adapters/cloudflare/src/index.tsRepository: marko-js/run
Length of output: 241
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- files ---'
git ls-files 'packages/adapters/cloudflare/scripts/importMetaURL.js' 'packages/adapters/cloudflare/scripts/build.ts' 'packages/adapters/cloudflare/src/index.ts'
echo
echo '--- importMetaURL.js ---'
cat -n packages/adapters/cloudflare/scripts/importMetaURL.js
echo
echo '--- build.ts matches ---'
rg -n 'import\.meta\.url|__importMetaURL|define' packages/adapters/cloudflare/scripts/build.ts packages/adapters/cloudflare/src/index.ts
echo
echo '--- index.ts relevant lines ---'
sed -n '1,120p' packages/adapters/cloudflare/src/index.tsRepository: marko-js/run
Length of output: 4145
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
const path = require('path');
const { pathToFileURL, fileURLToPath } = require('url');
const urlObj = pathToFileURL('/tmp/example.js');
console.log('pathToFileURL returns:', urlObj.constructor.name, urlObj.href);
for (const value of [urlObj, urlObj.href]) {
try {
const dirname = path.dirname(value);
console.log('path.dirname ok for', typeof value, value.constructor?.name ?? '(n/a)', '=>', dirname);
try {
console.log('fileURLToPath(path.dirname(...)) =>', fileURLToPath(dirname));
} catch (e) {
console.log('fileURLToPath failed:', e.code || e.name, e.message);
}
} catch (e) {
console.log('path.dirname failed for', typeof value, value.constructor?.name ?? '(n/a)', '=>', e.code || e.name, e.message);
}
}
JSRepository: marko-js/run
Length of output: 421
packages/adapters/cloudflare/scripts/importMetaURL.js: export a string URL
pathToFileURL(__filename) returns a URL object, and the CJS bundle passes that into path.dirname(), which throws ERR_INVALID_ARG_TYPE during module load. Export .href here so __importMetaURL stays a string.
🐛 Proposed fix
import { pathToFileURL } from "url";
-export const __importMetaURL = pathToFileURL(__filename);
+export const __importMetaURL = pathToFileURL(__filename).href;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { pathToFileURL } from "url"; | |
| export const __importMetaURL = pathToFileURL(__filename); | |
| import { pathToFileURL } from "url"; | |
| export const __importMetaURL = pathToFileURL(__filename).href; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/adapters/cloudflare/scripts/importMetaURL.js` around lines 1 - 2,
`__importMetaURL` is currently exporting a URL object from `importMetaURL.js`,
which causes `path.dirname()` to fail when the Cloudflare CJS bundle loads it.
Update the `__importMetaURL` export to be a string URL instead of the
`pathToFileURL(__filename)` object, keeping the existing `pathToFileURL` usage
but returning its `.href` so consumers like the module-load path resolution
logic receive the expected string value.
| async startPreview({ options: previewOptions }) { | ||
| assertWranglerCLI(); | ||
|
|
||
| const { port = 3000, cwd, dir } = previewOptions; | ||
| const publicDir = path.join(dir, "public"); | ||
|
|
||
| const args = | ||
| mode === "pages" | ||
| ? ["pages", "dev", publicDir, "--port", port.toString()] | ||
| : [ | ||
| "dev", | ||
| "--config", | ||
| path.join(dir, "wrangler.json"), | ||
| "--port", | ||
| port.toString(), | ||
| ]; | ||
|
|
||
| args.push(...parseWranglerArgs(previewOptions.args)); | ||
|
|
||
| const proc = spawn("wrangler", args, { | ||
| cwd, | ||
| env: process.env, | ||
| shell: true, | ||
| }); | ||
|
|
||
| if (process.env.NODE_ENV !== "test") { | ||
| proc.stdout.pipe(process.stdout); | ||
| } | ||
| proc.stderr.pipe(process.stderr); | ||
|
|
||
| return { | ||
| port, | ||
| close() { | ||
| proc.unref(); | ||
| proc.kill(); | ||
| }, | ||
| }; | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Preview in workers mode breaks when a root Wrangler config exists.
startPreview unconditionally points wrangler dev at path.join(dir, "wrangler.json"). But buildEnd (Lines 137-155) only writes that file when hasRootWranglerConfig is false — when a project already has its own root config (the explicitly supported "leave it untouched" scenario from the PR description), dist/wrangler.json is never created, and wrangler dev --config <missing-file> will fail.
🐛 Proposed fix
async startPreview({ options: previewOptions }) {
assertWranglerCLI();
const { port = 3000, cwd, dir } = previewOptions;
const publicDir = path.join(dir, "public");
+ const hasRootConfig = await hasRootWranglerConfig(cwd ?? process.cwd());
const args =
mode === "pages"
? ["pages", "dev", publicDir, "--port", port.toString()]
- : [
+ : hasRootConfig
+ ? ["dev", "--port", port.toString()]
+ : [
"dev",
"--config",
path.join(dir, "wrangler.json"),
"--port",
port.toString(),
];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async startPreview({ options: previewOptions }) { | |
| assertWranglerCLI(); | |
| const { port = 3000, cwd, dir } = previewOptions; | |
| const publicDir = path.join(dir, "public"); | |
| const args = | |
| mode === "pages" | |
| ? ["pages", "dev", publicDir, "--port", port.toString()] | |
| : [ | |
| "dev", | |
| "--config", | |
| path.join(dir, "wrangler.json"), | |
| "--port", | |
| port.toString(), | |
| ]; | |
| args.push(...parseWranglerArgs(previewOptions.args)); | |
| const proc = spawn("wrangler", args, { | |
| cwd, | |
| env: process.env, | |
| shell: true, | |
| }); | |
| if (process.env.NODE_ENV !== "test") { | |
| proc.stdout.pipe(process.stdout); | |
| } | |
| proc.stderr.pipe(process.stderr); | |
| return { | |
| port, | |
| close() { | |
| proc.unref(); | |
| proc.kill(); | |
| }, | |
| }; | |
| }, | |
| async startPreview({ options: previewOptions }) { | |
| assertWranglerCLI(); | |
| const { port = 3000, cwd, dir } = previewOptions; | |
| const publicDir = path.join(dir, "public"); | |
| const hasRootConfig = await hasRootWranglerConfig(cwd ?? process.cwd()); | |
| const args = | |
| mode === "pages" | |
| ? ["pages", "dev", publicDir, "--port", port.toString()] | |
| : hasRootConfig | |
| ? ["dev", "--port", port.toString()] | |
| : [ | |
| "dev", | |
| "--config", | |
| path.join(dir, "wrangler.json"), | |
| "--port", | |
| port.toString(), | |
| ]; | |
| args.push(...parseWranglerArgs(previewOptions.args)); | |
| const proc = spawn("wrangler", args, { | |
| cwd, | |
| env: process.env, | |
| shell: true, | |
| }); | |
| if (process.env.NODE_ENV !== "test") { | |
| proc.stdout.pipe(process.stdout); | |
| } | |
| proc.stderr.pipe(process.stderr); | |
| return { | |
| port, | |
| close() { | |
| proc.unref(); | |
| proc.kill(); | |
| }, | |
| }; | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/adapters/cloudflare/src/index.ts` around lines 68 - 105, The
workers-mode preview path is hardcoded to use a generated wrangler.json, which
breaks when a root Wrangler config already exists and buildEnd does not create
that file. Update startPreview to choose the config source based on the same
hasRootWranglerConfig/buildEnd logic, so wrangler dev is pointed at an existing
config or skips --config when the project’s own root config should be used. Keep
the behavior aligned between buildEnd and startPreview by using the shared
mode/hasRootWranglerConfig branching.
| const proc = spawn("wrangler", args, { | ||
| cwd, | ||
| env: process.env, | ||
| shell: true, | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Node.js child_process spawn shell true Windows argument quoting paths with spaces
💡 Result:
When using child_process.spawn with { shell: true } on Windows, Node.js effectively concatenates the command and its arguments into a single string that is then executed by the system shell (typically cmd.exe) [1][2]. Because this process simply joins the array elements with spaces, it does not automatically handle or escape paths that contain spaces [1][2]. To successfully spawn a command with paths containing spaces when shell: true is enabled: 1. Manual Quoting: You must manually wrap paths or arguments containing spaces in double quotes [3]. For example: spawn('"C:\Path With Spaces\my-executable.exe"', ['"arg with spaces"'], { shell: true }); 2. Unified String Approach: Since passing an arguments array when shell: true is discouraged (and subject to deprecation warnings) due to security risks and improper handling, a common and recommended alternative is to pass a single, properly formatted string as the first argument to spawn [4][2]. spawn('"C:\Path With Spaces\my-executable.exe" "arg with spaces"', { shell: true }); 3. Avoid shell: true: Whenever possible, avoid using { shell: true }. Without the shell option, Node.js handles argument quoting automatically and securely on Windows, allowing you to pass paths with spaces in the arguments array without manual escaping [5][6]. Security Warning: When shell: true is enabled, never pass unsanitized user input to the command or arguments. Because the shell interprets the resulting string, any input containing shell metacharacters (like &, |, or >) can be exploited to execute arbitrary commands [5][7]. If you require a shell for features like piping or redirection, ensure all inputs are strictly validated or escaped [5][6].
Citations:
- 1: child_process should individually escape args[] on shell: true nodejs/node#29532
- 2: DEP0190 (passing args to spawn with shell: true) isn't fixable when
stdiooption is required nodejs/node#58763 - 3: Cannot spawn shell script if path has spaces nodejs/node#38490
- 4: Disallow args in child_process execFile/spawn when the shell option is true nodejs/node#57143
- 5: https://nodejs.org/api/child_process.html
- 6: https://stackoverflow.com/questions/48014957/quotes-in-node-js-spawn-arguments
- 7: https://nodejs.org/docs/latest-v26.x/api/child_process.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## outline\n'
ast-grep outline packages/adapters/cloudflare/src/index.ts --view expanded || true
printf '\n## relevant lines\n'
sed -n '1,180p' packages/adapters/cloudflare/src/index.ts | cat -n
printf '\n## wrangler searches\n'
rg -n 'spawn\("wrangler"|shell: true|publicDir|config' packages/adapters/cloudflare/src/index.ts packages/adapters/cloudflare/src -S || trueRepository: marko-js/run
Length of output: 11349
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,180p' packages/adapters/cloudflare/src/index.ts | cat -nRepository: marko-js/run
Length of output: 6698
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '180,280p' packages/adapters/cloudflare/src/index.ts | cat -nRepository: marko-js/run
Length of output: 1866
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## wrangler references\n'
rg -n '"wrangler"|wrangler' package.json packages -g 'package.json' -g 'pnpm-lock.yaml' -g 'package-lock.json' -g 'yarn.lock' -S || true
printf '\n## wrangler package docs if present\n'
fd -a 'wrangler*' . | head -n 50 || trueRepository: marko-js/run
Length of output: 211
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## package files\n'
git ls-files 'packages/**/package.json' 'package.json' | sed -n '1,200p'
printf '\n## cloudflare package manifest\n'
sed -n '1,220p' packages/adapters/cloudflare/package.json | cat -nRepository: marko-js/run
Length of output: 1915
Remove shell: true from the Wrangler spawn. spawn already passes publicDir, --config, and the filtered Wrangler args as argv entries, so shell mode is unnecessary here and can break paths with spaces on Windows.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/adapters/cloudflare/src/index.ts` around lines 87 - 91, The Wrangler
process launch in spawn should not use shell mode. Update the spawn call in the
proc creation path to remove shell: true so Wrangler receives the publicDir,
--config, and filtered args directly as argv entries; keep the existing cwd and
env handling in place.
|
Splitting this into one PR per adapter for easier review:
Closing this combined PR in favor of those. Generated by Claude Code |
1 participant
Description
Adds a new adapter package,
@marko/run-adapter-cloudflare, for previewing and deploying Marko Run apps to Cloudflare. It supports both Cloudflare targets via amodeoption:mode: "workers"(default) — builds a bundled_worker.js(export default { fetch }) for Cloudflare Workers with a static assets binding, and generates a starterwrangler.jsonpointing at the worker and thepublic/assets. An existing project-level Wrangler config (wrangler.toml/.json/.jsonc) is left untouched so user-defined bindings/secrets/name are preserved.mode: "pages"— builds a Cloudflare Pages "advanced mode"_worker.jsalongside the static assets and emits a_routes.jsonthat excludes the static asset directory from invoking the function.Implementation mirrors the existing Netlify edge adapter:
webworkerwithworkerd/workerexport conditions andnoExternal: true(single self-contained bundle).default-entrythat calls@marko/run's router and falls back toenv.ASSETS.fetchwhen no route matches.{ env, ctx, cf }via the exportedCloudflarePlatformInfotype.wrangler devfor Workers,wrangler pages devfor Pages).Also adds two test fixtures to the
@marko/runpackage (cloudflare-adapter-workersandcloudflare-adapter-pages) mirroring the Netlify fixtures, and a changeset for the new package.Motivation and Context
Cloudflare is one of the most-requested deployment targets and the canonical edge platform, but Marko Run shipped no adapter for it — users had to wire up Workers/Pages by hand. Because the runtime is built on web standards, a Cloudflare adapter is a natural fit and fills the largest gap in deployment coverage alongside the existing Node, static, and Netlify adapters.
Screenshots (if appropriate):
Checklist:
Generated by Claude Code