Repository navigation
feat(init): add --name to set the Cloudflare Worker name - #3863
Conversation
vinext init derives the Worker name from package.json, so tools that run it on a user's behalf, such as @cloudflare/autoconfig, cannot apply a name the user chose. --name sets the Worker name in generated cloudflare.config.ts or wrangler.jsonc and in the names derived from it. It fails instead of being ignored when the config that holds the name already exists.
@vinext/cloudflare
create-vinext-app
@vinext/types
vinext
@cloudflare/workers-response-store
commit: |
Performance benchmarksCompared 0 improved · 0 regressed · 7 within ±1.5%
View detailed results and traces 🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head |
|
|
/bigbonk review for issues |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
| undefined, | ||
| '--legacy-wrangler-cloudflare-init expects true or false when using the "--legacy-wrangler-cloudflare-init=value" form.', | ||
| ); | ||
| const name = parseNameArg(args); |
There was a problem hiding this comment.
resolveInitOptions is also used by runCreateVinextAppCli, but that CLI’s positional scanner does not consume the new space-separated value. As a result, create-vinext-app --name storefront actual-app ... sets parsed.directory to storefront and silently ignores actual-app, while still using storefront as the Worker name. Please add --name to the value-taking flags in packages/create-vinext-app/src/index.ts (and cover this invocation) so the documented --name <name> form works through the shared init flow.
There was a problem hiding this comment.
Fixed in 477c498: create-vinext-app now skips the value after --name when it looks for the app directory, and its --help lists the flag. A new test in tests/create-vinext-app.test.ts runs --name storefront <appPath> ... and checks that cloudflare.config.ts is generated in <appPath> with name: "storefront", and that no storefront directory is created. The test fails on the previous head and passes on this one. The whole file passes (17 tests).
|
Review: 1 findings. Posted 1 inline finding. Targeted tests passed: 263 tests across 3 files. |
…rectory Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/bigbonk review for issues |
| @@ -0,0 +1,33 @@ | |||
| { | |||
There was a problem hiding this comment.
The entire top-level storefront/ tree is generated create-vinext-app output from the regression reproduction, and it is neither included by pnpm-workspace.yaml nor referenced by the test suite. Keeping it adds 12 untested application files that can become stale independently of the actual template. Please remove the whole storefront/ directory; the new temp-directory test already preserves this reproduction.
There was a problem hiding this comment.
Removed in f3f9566. That directory was generated by mistake when the new test was run against the old parser, which treated storefront as the app directory relative to the repo root. The test itself only writes to its temp directory.
|
Since last review: 1 resolved, 0 still open, 1 new. Posted 1 inline finding. Targeted tests passed: 264 tests across 3 files. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/bigbonk review for issues |
| if (arg.startsWith("-")) { | ||
| if ( | ||
| arg === "--platform" || | ||
| arg === "--name" || |
There was a problem hiding this comment.
The CLI now supports --name, but the local CloudflareInitOptions used by the exported createVinextApp() signature still has no name?: string. Because this package publishes dist/index.d.ts, TypeScript consumers of the programmatic API cannot request the same custom Worker name: an object literal under initOptions.cloudflare is rejected even though the runtime shared type supports it. Please add name?: string here (or reuse the shared exported option type) and cover the programmatic call.
There was a problem hiding this comment.
Fixed in 1654f77: create-vinext-app's CloudflareInitOptions now has name?: string, matching the shared type. A new test calls createVinextApp() with an inline cloudflare: { ..., name: "storefront" } literal and checks the generated cloudflare.config.ts. Without the type change, vp check rejects that literal with TS2353. The CLI test now also mocks process.cwd() to its temp directory, so a parser regression can't write into the repo. All 18 tests in the file pass.
|
Since last review: 1 resolved, 0 still open, 1 new. Posted 1 inline finding about the public |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/bigbonk review for issues |
| legacyWrangler?: boolean; | ||
| prerender?: boolean; | ||
| /** Worker name for generated config, instead of one derived from package.json. */ | ||
| name?: string; |
There was a problem hiding this comment.
The newly supported programmatic path does not pass through parseNameArg, so this public option can still contain uppercase characters, spaces, leading/trailing dashes, or more than 63 characters. createVinextApp() forwards initOptions directly to init(), and init() uses the value to generate the Worker config and Response Store resource/script names without validating it. Please move/reuse the Worker-name validation at the shared init boundary (before any files are changed) and cover invalid programmatic input; otherwise the stated Worker-name contract only holds for CLI callers.
There was a problem hiding this comment.
Fixed in df0ae3c. The Worker-name check is now a shared validateWorkerName() in init-platform.ts. parseNameArg uses it, and so does validateCloudflarePlatformSetup, which runs before init() changes any files. That covers programmatic callers like createVinextApp(). A new tests/init.test.ts case passes "", Storefront, store front, -storefront and a 64-character name through init(). Each one is rejected and leaves the project unchanged. All 5 cases fail without the boundary check. tests/init.test.ts (193 tests) and tests/init-platform.test.ts (59) pass.
|
Review: 1 findings. Posted 1 actionable inline finding. No branch changes made. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/bigbonk review for issues |
|
Since last review: 1 resolved, 0 still open, 0 new. LGTM! Targeted tests passed: 270 tests across 3 files. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/bigbonk review for issues |
| warmCdnCache: options.cloudflare?.warmCdnCache ?? false, | ||
| legacyWrangler: options.cloudflare?.legacyWrangler, | ||
| scriptNames: options.scriptNames, | ||
| workerName: options.cloudflare?.name, |
There was a problem hiding this comment.
The existing cloudflare.config.ts is now authoritative and the warning says --name was not applied, but this still forwards the requested name to addScripts(). With Response Store service-binding enabled and no existing deploy:response-store script, line 278 therefore creates a command targeting <requested-name>-response-store even though the retained config exports a differently named auxiliary Worker. That command will deploy the wrong/nonexistent build target. Please avoid using --name for this script when configKeepingWorkerName is set (or derive the effective auxiliary name from the existing config), and cover this existing-config + Response Store combination.
There was a problem hiding this comment.
Fixed in c28d97e. When an existing config keeps its Worker name, init() no longer passes --name to addScripts(), so deploy:response-store uses the derived name, as it did before --name existed. A new test in tests/init.test.ts covers an existing cloudflare.config.ts + Response Store service-binding + --name. It asserts the exact script, and on the previous head it fails with --worker storefront-production-response-store. All 194 tests in the file pass.
|
Review: 1 findings. Posted 1 inline finding. Targeted tests passed: 270 tests across 3 files. |
… not applied Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/bigbonk review for issues |
| responseStoreMode?: InitResponseStoreMode; | ||
| warmCdnCache?: boolean; | ||
| legacyWrangler?: boolean; | ||
| /** Worker name for generated config, instead of one derived from package.json. */ |
There was a problem hiding this comment.
The programmatic API now accepts name, but createVinextApp() calls writeTemplate() before init() reaches validateWorkerName(). An invalid name therefore creates the full project and then throws, leaving a non-empty directory that blocks a corrected retry. Please validate this option before writeTemplate() (and cover that invalid input leaves the target absent or empty), so the public API has the same preflight guarantee as direct init().
There was a problem hiding this comment.
Fixed in 45f4af0. createVinextApp() now validates initOptions.cloudflare.name with the shared validateWorkerName() alongside its other preflight checks, before writeTemplate(). A new test passes name: "Storefront" and asserts that the call rejects and the target directory doesn't exist. On the previous head that test fails because the directory gets created. All 19 tests in tests/create-vinext-app.test.ts pass.
|
Since last review: 1 resolved, 0 still open, 1 new. Posted 1 actionable inline finding. No branch changes made. |
…emplate Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/bigbonk review for issues |
| } | ||
| // init() validates this too, but only after the template is written. | ||
| const workerName = options.initOptions.cloudflare?.name; | ||
| if (workerName !== undefined) validateWorkerName(workerName); |
There was a problem hiding this comment.
This preflight validates the name syntax, but it still accepts a valid name when initOptions.platform is "node". The public ResolvedInitOptions shape permits { platform: "node", cloudflare: { ..., name: "storefront" } }; writeTemplate() then creates the full Node project and init() silently ignores the Cloudflare options, whereas the CLI rejects the same combination with --name requires --platform=cloudflare. Please reject this combination before writing the template, and enforce the same invariant at the shared init() boundary for direct programmatic callers.
There was a problem hiding this comment.
Declining this one. When init() and createVinextApp() are called with platform: "node", they already ignore every cloudflare option, including legacyWrangler, which the CLI also rejects on Node. name follows the same rule. The --platform=cloudflare requirement is enforced where options are resolved from flags (resolveInitOptions), as it is for --legacy-wrangler-cloudflare-init. Enforcing it for programmatic callers would change how all Cloudflare options are handled, so it's out of scope for this PR.
|
Since last review: 1 resolved, 0 still open, 1 new. Posted 1 inline finding. All 272 targeted tests passed. |
|
/bigbonk review for issues |
|
Since last review: 1 resolved, 0 still open, 0 new. LGTM! |
2 participants
Summary
vinext initderives the Worker name frompackage.json(or the directory name). Tools that run it on a user's behalf have no way to pass a name the user chose. This came up in cloudflare/workers-sdk#16200, where@cloudflare/autoconfigrunsvinext initfor Next.js 16 projects targetingcf: autoconfig lets the user pick a Worker name (and honoursWRANGLER_CI_OVERRIDE_NAMEin Workers Builds), but the generatedcloudflare.config.tsalways used vinext's derived name.This adds
--name <name>tovinext init:cloudflare.config.ts(orwrangler.jsoncwith--legacy-wrangler-cloudflare-init), for the names derived from it (the Response Store Worker and its R2 bucket), and in thedeploy:response-storescript.--platform=cloudflare, in the same way as--legacy-wrangler-cloudflare-init.cloudflare.config.ts, orwrangler.jsonc/wrangler.jsonin legacy mode), init keeps the Worker name in that file and prints a warning that--namewas not applied, since that file may already use the name. Thedeploy:response-storescript then keeps its derived name too, so it doesn't target a Worker the kept config doesn't define.The flag name matches
--nameon@vinext/cloudflare deploy. The README andvinext init --helpdocument it.Tests
tests/create-vinext-app.test.ts):--name <name>before the app directory, andnamethrough the programmaticcreateVinextApp()API.tests/init-platform.test.ts): both flag forms, missing values, invalid names including the 63-character limit, and rejection outside Cloudflare init.tests/init.test.ts): the name is used incloudflare.config.tsand legacywrangler.jsonc; the Response Store Worker anddeploy:response-storescript are named after it; init warns and keeps the existing config's Worker name, in both modes; and invalid names passed programmatically are rejected before any files change.create-next-appproject with--name=storefront-productionand checked the generatedcloudflare.config.ts, and the invalid-name error. (This ran when an existing config was still an error; that case is now a warning, whichtests/init.test.tscovers.)Merge order
This is one of three related changes:
@cloudflare/autoconfig, but it must be incfbefore step 4, otherwise the firstcf devafter setup runsnext devrather thanvite dev.--nametovinext init) merges and is released. (this PR)cftarget) is updated to pass the Worker name chosen during setup tovinext init --nameand to require the vinext release from step 2. It then merges and@cloudflare/autoconfigis released.cfbumps@cloudflare/autoconfigto the release from step 3, after fix: analyse the project again after framework setup cf#260 has merged.