feat(wrangler): Add a new wrangler preview command group for Worker Previews#12983
feat(wrangler): Add a new wrangler preview command group for Worker Previews#129831000hz wants to merge 9 commits intocloudflare:mainfrom
wrangler preview command group for Worker Previews#12983Conversation
🦋 Changeset detectedLatest commit: 15da67f 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 |
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
f9cf745 to
2ab5201
Compare
create-cloudflare
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-editor-shared
wrangler
commit: |
f165a2b to
14993e8
Compare
e4942f6 to
a65531f
Compare
penalosa
left a comment
There was a problem hiding this comment.
First pass review—my main concern at this stage is code duplication, and using existing utilities from across the codebase.
| }); | ||
| }); | ||
|
|
||
| describe("[previews]", () => { |
There was a problem hiding this comment.
This is fairly minimal testing. I'd like to see tests of the inheritability of the previews field and how values in the previews field are parsed.
| wrangler dispatch-namespace 🏗️ Manage dispatch namespaces | ||
| wrangler init [name] 📥 Initialize a basic Worker | ||
| wrangler pages ⚡️ Configure Cloudflare Pages | ||
| wrangler preview [script] 👀 Create a Preview deployment of the current Worker [private beta] |
There was a problem hiding this comment.
Can you talk me through your thinking around plural/non-plural previews? It seems to show up in both forms in various places (e.g. Workers Previews)
There was a problem hiding this comment.
wrangler preview <= "preview" used in a verb sense, as its a command; analogous to wrangler deploy
"[Create a] Preview" <= a capital-P Preview resource, a new first-class platform entity
"Preview deployment" <= a deployment of a particular Preview resource
"Previews settings" <= The distinct set of configuration the platform holds for your Worker for all of its Previews; alternative to its "Production settings"
"previews" <= the new config file field which is used to provide config for deployments made with wrangler preview, and used to update your Previews settings with wrangler preview settings update
"Workers Previews" <= nothing is ever referred to in this way
"Worker Previews" <= the name of this platform feature
It's admittedly a lot, but we've strived to be consistent in our usage across our platform and user-facing surfaces. Please raise if you see something that doesn't adhere to this, i.e. referring to a Preview resource as a "preview", etc.
| * Get a human-readable display value for a binding. | ||
| * Used by both preview deployment and Previews settings formatting. | ||
| */ | ||
| export function getBindingValue(binding: Binding): string { |
There was a problem hiding this comment.
We have an existing utility for this that should be used instead: printBindings()
There was a problem hiding this comment.
printBindings() prints all bindings straight to the logger, which limits any control over formatting, etc. (perhaps somewhat intentionally).
I agree that we should not be duplicating this type of logic across the codebase, but I think we need a more flexible utility available for this sort of thing. This really ought to belong in @cloudflare/workers-utils and be consumed by printBindings().
There was a problem hiding this comment.
perhaps somewhat intentionally
It is intentional, yes—it means that every time Wrangler prints out bindings they're formatted the same. Are there specific aspects of the printBindings() format that you'd like to change? Or is this primarily about being able to wrap the output in a box?
| type: "string", | ||
| requiresArg: true, | ||
| }, | ||
| "ignore-defaults": { |
There was a problem hiding this comment.
We've discussed this at length, I know, but I'm just restating my opinion that users will find this confusing, and that there's a real potential for ending up with accidentally deployed previews with very stale config values from the dash. It also behaves completely differently to everything else in Wrangler.
There was a problem hiding this comment.
This seems like it has a lot of duplicated code from the deployment pipeline?
There was a problem hiding this comment.
as far as I could tell, there weren't low-level enough utilities we could reuse here, as the deployment pipeline code assumes it needs to prepare a multipart form-data request, whereas we need to hit the /workers/workers/... endpoints which only accept JSON, and have different schema requirements, etc.
Again, I'd love to not have to duplicate any logic here, and hope we can refactor things to share the core functionality.
… Previews - Implement a `wrangler preview` preview command for creating new Preview deployments - Implement Previews settings management with `wrangler preview settings` and `wrangler preview settings update` - Implement Previews secrets management with `wrangler preview secret put/delete/list/bulk` - Add config validation support for previews in workers-utils - Add tests for preview commands, settings merge behavior, and config validation - Add shared CLI formatting/utility helpers used by preview output
dffb6fc to
9830a63
Compare
| isValid = | ||
| validateRequiredProperty( | ||
| diagnostics, | ||
| `${field}.limits`, | ||
| "cpu_ms", | ||
| previews.limits.cpu_ms, | ||
| "number" | ||
| ) && isValid; |
There was a problem hiding this comment.
🟡 validatePreviewsConfig makes cpu_ms required in previews.limits while it's optional everywhere else
When a user provides a limits object inside previews, the validation uses validateRequiredProperty for cpu_ms (line 5062). This means configs like previews: { limits: {} } or previews: { limits: { subrequests: 10 } } will fail validation with an error saying cpu_ms is required. In contrast, the normal normalizeAndValidateLimits at packages/workers-utils/src/config/validation.ts:4702-4728 uses validateOptionalProperty for both cpu_ms and subrequests, making them optional. This inconsistency means valid limit configurations are rejected when placed inside the previews block. Additionally, subrequests is not validated at all inside previews limits (neither as optional nor flagged as an unexpected property).
| isValid = | |
| validateRequiredProperty( | |
| diagnostics, | |
| `${field}.limits`, | |
| "cpu_ms", | |
| previews.limits.cpu_ms, | |
| "number" | |
| ) && isValid; | |
| if (previews.limits !== undefined) { | |
| isValid = | |
| validateOptionalProperty( | |
| diagnostics, | |
| `${field}.limits`, | |
| "cpu_ms", | |
| previews.limits.cpu_ms, | |
| "number" | |
| ) && isValid; | |
| isValid = | |
| validateOptionalProperty( | |
| diagnostics, | |
| `${field}.limits`, | |
| "subrequests", | |
| previews.limits.subrequests, | |
| "number" | |
| ) && isValid; | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
…eploy - Add preview.yml: creates a Worker Preview on every PR using wrangler preview (prerelease from cloudflare/workers-sdk#12983), posts/updates a PR comment with the preview URL - Add deploy.yml: runs npm run deploy on pushes to main - Update wrangler to prerelease version from pkg.pr.new/wrangler@12983 - Replace deploy:preview script to call wrangler preview after build - Remove vite preview script (superseded by wrangler preview)
* Add GitHub Actions workflows for preview deployments and production deploy - Add preview.yml: creates a Worker Preview on every PR using wrangler preview (prerelease from cloudflare/workers-sdk#12983), posts/updates a PR comment with the preview URL - Add deploy.yml: runs npm run deploy on pushes to main - Update wrangler to prerelease version from pkg.pr.new/wrangler@12983 - Replace deploy:preview script to call wrangler preview after build - Remove vite preview script (superseded by wrangler preview) * Fix: install wrangler prerelease as a workflow step, not in package.json npm ci rejects non-semver versions (URL-based installs), so the prerelease wrangler must be installed after npm ci rather than declared in package.json. * Fix: restore clean package-lock.json without pkg.pr.new URLs The lockfile had URL-resolved entries for wrangler and its sub-deps from the prerelease install, which caused npm ci to fail with 'Invalid Version'. * Fix: capture wrangler output before propagating exit code - Use set +e to prevent bash -e from aborting before we can echo output - Run Post preview comment step with if: always() so it posts even on failure * Fix: don't post PR comment on failure, fix output interpolation in github-script - Remove if: always() so comment only posts on successful deploy - Remove raw wrangler output interpolation into JS template literal which caused SyntaxError when output contained JS keywords * Fix: add CLOUDFLARE_ACCOUNT_ID env var to both workflows Wrangler fails in non-interactive mode when multiple accounts are available and no account ID is specified. * Fix: pass branch name as preview --name to avoid detached HEAD slug GitHub Actions checks out PRs in detached HEAD state, causing git rev-parse --abbrev-ref HEAD to return 'HEAD' -> slug 'head'. Pass --name explicitly using github.head_ref instead.
Refs IAC-362
This PR introduces a
wrangler previewfamily of commands for creating Preview deployments and managing Previews settings. This feature is currently limited to private beta status.New commands
wrangler previewwrangler preview deletewrangler preview settingswrangler preview settings updatewrangler preview secret putwrangler preview secret deletewrangler preview secret listwrangler preview secret bulkIt also introduces a
previewsproperty to the Wrangler config schema which includes all binding-related fields as well aslimits,observability, andlogpush. These fields all adhere to their existing heritability traits, withpreviewsitself being inheritable. This property allows users to specify an alternate configuration for their Worker's Preview deployments.Also included are some new box drawing utilities, though we may want to swap these out for a library like https://github.com/sindresorhus/boxen in the future.