Type command values from declared option presets#4
Merged
Conversation
Each command's run body typed its `values` shape by hand, re-listing the flags
its option presets ("ns", "control", "json", ...) expand to. That duplicated
CLI_OPTION_PRESETS in common.js and could silently drift from it -- a preset
gaining or dropping a flag would not surface in the handler's type.
Add a `PresetFlags<P>` typedef to lib/command.js: `PresetValueMap` records the
`values` each preset contributes (kept beside the preset list it mirrors), and
`PresetFlags<"ns" | "control" | "json">` intersects a chosen set via a
`UnionToIntersection` helper. Commands now name their presets and intersect
only their own defineCliOption flags inline, e.g.
`PresetFlags<"ns" | "control" | "json"> & { worker?: string, yes?: boolean }`.
This is type-only: no runtime change, zero `any`, and the preset-contributed
portion of every handler's `values` is now derived from one source instead of
restated per command. Converted config, d1, delete, deploy, doctor, r2, secret,
tail, token, whoami, and workflows; workers reads `context.values` (no typed
binding) and init uses its own parse shell, so both are untouched.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Lu Zhang <lu@wdl.dev>
There was a problem hiding this comment.
Pull request overview
This PR centralizes the JSDoc typing for command values by deriving preset-contributed flag shapes from a single map, reducing per-command duplication and preventing preset/type drift as shared option presets evolve.
Changes:
- Add
PresetValueMap,UnionToIntersection, andPresetFlags<P>typedefs inlib/command.jsto model the parsedvalueseach option preset contributes. - Update command handlers’
valuesparameter types to usePresetFlags<...>intersected with command-specific flags. - Align preset typing with the current preset set (including
"control"carrying--no-token-storeand the"endpoint"preset).
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| lib/command.js | Introduces PresetValueMap + PresetFlags<P> to type preset-expanded values from one source. |
| commands/config.js | Switches values typing to PresetFlags<"ns" | "control" | "json">. |
| commands/d1.js | Replaces the hand-written flags typedef with PresetFlags<...> + D1-specific flags. |
| commands/delete.js | Uses PresetFlags<"ns" | "control" | "json"> for shared flags and keeps delete-specific flags inline. |
| commands/deploy.js | Uses PresetFlags<"ns" | "control"> to include shared auth/namespace flags alongside deploy flags. |
| commands/doctor.js | Switches values typing to the relevant preset-derived shape. |
| commands/r2.js | Uses PresetFlags<"ns" | "control" | "json"> plus R2-specific flags. |
| commands/secret.js | Uses PresetFlags<"ns" | "control" | "json"> plus secret-specific flags. |
| commands/tail.js | Uses PresetFlags<"ns" | "control"> plus tail-specific flags. |
| commands/token.js | Uses PresetFlags<"endpoint"> to type --control-url while keeping token subcommand flags explicit. |
| commands/whoami.js | Switches values typing to PresetFlags<"ns" | "control" | "json">. |
| commands/workflows.js | Uses PresetFlags<"ns" | "control" | "json"> plus workflows-specific flags. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
`runWorkers` was the last defineCommand handler still reading the json flag off the untyped `context.values` (Record<string, unknown>), leaving its ["ns", "control", "json"] schema uncovered by the typed preset composition. Bind a typed `values: PresetFlags<"ns" | "control" | "json">` in the run body and thread the json boolean into printWorkersList as a parameter, so the read goes through the derived type instead of the loose context bag. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Lu Zhang <lu@wdl.dev>
Reword the parenthetical so it no longer says "--control preset" (the preset is the string "control"; there is no --control flag) and separates preset names from the parsed `values` keys they map to. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Lu Zhang <lu@wdl.dev>
The clarification repeated "keyed by preset name"; collapse it to one line. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Lu Zhang <lu@wdl.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds a
PresetFlags<P>typedef tolib/command.jsand uses it across the command handlers so each command'svaluestype derives the preset-contributed flags from a single source instead of restating them per command.Why
Every command's run body typed its
valuesshape by hand, re-listing the flags its option presets ("ns","control","json", ...) expand to. That duplicatedCLI_OPTION_PRESETSincommon.jsand could silently drift from it — a preset gaining or dropping a flag (e.g.--controlpicking up--no-token-store) would not surface in any handler's type.How
PresetValueMaprecords thevalueseach preset contributes, kept beside the preset-name doc it mirrors.PresetFlags<"ns" | "control" | "json">intersects the chosen presets via aUnionToIntersectionhelper.defineCliOptionflags inline, e.g.PresetFlags<"ns" | "control" | "json"> & { worker?: string, yes?: boolean }.Type-only change: no runtime behavior change, zero
any. Convertedconfig,d1,delete,deploy,doctor,r2,secret,tail,token,whoami,workflows.workersreadscontext.values(no typed binding) andinituses its own parse shell, so both are untouched.Checks
npm run typecheck— cleannpm run lint— cleannpm test— 379/379 pass🤖 Generated with Claude Code
Generated by Claude Code