fix(cloudflare): split account-or-zone scoped operations into per-scope variants#282
Merged
Merged
Conversation
…pe variants
Cloudflare's API has paths like /{accountOrZone}/{accountOrZoneId}/... where
the first segment is "accounts" or "zones" depending on which scope ID the
caller provides. The OpenAPI spec collapses both into one entry with
account_id and zone_id both declared required (mutually exclusive in reality).
The generator was producing uncallable code for ~85 endpoints across 7
services as a result.
This change makes the generator detect that path pattern and emit two concrete
operations (<op>ForAccount, <op>ForZone) plus a thin wrapper (<op>) that
dispatches on "accountId" in input. The wrapper's input is the discriminated
union so invalid states are unrepresentable. Body fields are extracted into a
shared per-operation BaseFields const so source size is bounded.
Also includes two adjacent generator fixes:
- synthesize URL path params declared in the URL template but missing from the
parameters list (e.g. rulesetPhase)
- emit optional path params as Schema.optional(...) instead of treating them
as required
Supersedes #257 which used a runtime trait + throw for the same problem.
Contributor
|
Install the packages built from this commit: @distilled.cloud/cloudflare bun add https://pkg.distilled.cloud/cloudflare/db50d64 |
Hoist the non-scope fields into a single `<Op>BaseRequest` interface and have each variant interface extend it with just its scope field. Avoids duplicating large body type declarations across both variants — for ops like PutPhasRequest with multi-thousand-line bodies this cuts the emitted file size meaningfully without changing the public type surface.
sam-goodwin
reviewed
May 7, 2026
Comment on lines
+458
to
+460
| "accountId" in input | ||
| ? listAccessRulesForAccount.pages(input) | ||
| : listAccessRulesForZone.pages(input); |
Collaborator
There was a problem hiding this comment.
should probably be an error if both are provided? ambiguity?
Collaborator
There was a problem hiding this comment.
why are you calling .pages here?
what about yield* listAccessRules .items, .pages
…rZone variants The wrapper functions (`putPhas`, etc.) and `<Op>Request` union type aliases were convenience surface that hid which scope the call addresses behind a runtime `"accountId" in input` check. Drop them — callers now pick `<op>ForAccount` or `<op>ForZone` explicitly. This: - removes ~85 wrapper functions and their union type aliases - lets each variant remain a clean `API.OperationMethod` value with full type metadata (no plain-function intermediary) - removes the conditional `Effect`/`Stream` import requirement triggered only by wrappers
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
2 participants
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.
Cloudflare's API has a path pattern
/{accountOrZone}/{accountOrZoneId}/...where the literal first segment isaccountsorzonesdepending on which scope ID the caller provides. The OpenAPI spec collapses both into one entry that declaresaccount_idandzone_idas both required path params (mutually exclusive in reality). Distilled's generator was faithfully turning this into broken code: both fields typed as required, and{accountOrZone}/{accountOrZoneId}placeholders never bound to anything, so ~85 endpoints across 7 services were uncallable.This PR fixes those endpoints by splitting each at codegen time into two concrete operations plus a thin convenience wrapper. Supersedes #257, which used a runtime trait +
throw new Errorfor the same problem.Generator changes
Three independent fixes in
packages/cloudflare/scripts/generate.ts:Auto-detect
accountOrZone-scoped operations by URL template and split each into:<op>ForAccount— concrete URL/accounts/{account_id}/..., request schema withaccountId<op>ForZone— concrete URL/zones/{zone_id}/..., request schema withzoneId<op>— wrapper function dispatching on"accountId" in input. ReturnsEffect.Effect<...>(orstream.Stream<...>for paginated). Input is the discriminated union, so invalid states are unrepresentable at the type level.<Op>BaseFieldsconst and spread into both variants — no source-level duplication.Synthesize missing URL path params. Some operations reference placeholders like
{rulesetPhase}in the URL but don't declare them as parameters — these are now auto-synthesized as required string path params.Optional path params now correctly emit as
Schema.optional(...)instead of being typed as required.User-visible API
Before (broken — no way to substitute
accountOrZone/accountOrZoneId, both ids typed required):After:
Why split at codegen instead of runtime
Considered a runtime approach (PR #257): mark the schema with a trait, rewrite the path in
transformRequestParts, throw on invalid input. Rejected because (a) the throw bypasses Effect's typed error channel — it becomes an unchecked defect rather than a typed operation error, (b) "exactly one of" is encodable in the type system and shouldn't be a runtime check, and (c) it imports Cloudflare-specific path-rewriting logic into the core client. Splitting at codegen keeps core untouched, errors flow through Effect normally, and the type system enforces the invariant.Test coverage
Added in
packages/cloudflare/test/client-api.test.ts:putPhasForAccountbuilds/accounts/{id}/...correctlyputPhasForZonebuilds/zones/{id}/...correctlyaccountIdis presentzoneIdis present"accountId" in inputAll 7 client-api tests pass;
typecheckandcheckare clean for both the cloudflare package and core.Out of scope (separate issues to file)
getPhas,putPhas,listPhasVersions, etc.) — these come from Cloudflare's own published OpenAPI spec and need a spec patch, not a generator fix.buildRequestPartsto handleSchema.Unionnatively (would let SDKs express discriminated request unions generically without codegen-time splitting).