Skip to content

fix: use nonempty() for retry array type to match runtime validation#1199

Open
claygeo wants to merge 2 commits into
quirrel-dev:mainfrom
claygeo:fix/enqueue-retry-type
Open

fix: use nonempty() for retry array type to match runtime validation#1199
claygeo wants to merge 2 commits into
quirrel-dev:mainfrom
claygeo:fix/enqueue-retry-type

Conversation

@claygeo

@claygeo claygeo commented Mar 27, 2026

Copy link
Copy Markdown

Problem

EnqueueJobOptions.retry is typed as (number | string)[] which allows empty arrays, but the Zod schema at src/client/index.ts:132 enforces .min(1) at runtime. Passing an empty retry array fails with:

body/retry must NOT have fewer than 1 items

The type doesn't reflect this constraint because Zod's .min() does not change the inferred type.

Solution

Replace .min(1) with .nonempty(), which both:

  • Enforces the minimum at runtime (same behavior)
  • Narrows the inferred TypeScript type to [T, ...T[]] (at least one element)

The .max(10) constraint is preserved.

- retry: z.array(timeDuration("retry")).min(1).max(10).optional(),
+ retry: z.array(timeDuration("retry")).nonempty().max(10).optional(),

Changes

  • src/client/index.ts — Replaced .min(1) with .nonempty() on retry schema

Closes #1167

The Zod schema enforces .min(1).max(10) on the retry array, but
.min() does not change the inferred TypeScript type. This means
the type allows empty arrays while the runtime rejects them with
"body/retry must NOT have fewer than 1 items".

Replace .min(1) with .nonempty() which both enforces the minimum
at runtime AND narrows the inferred type to require at least one
element, aligning the TypeScript type with the runtime behavior.

Closes quirrel-dev#1167
@netlify

netlify Bot commented Mar 27, 2026

Copy link
Copy Markdown

👷 Deploy request for quirrel-development-ui pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit bd9ff32

@netlify

netlify Bot commented Mar 27, 2026

Copy link
Copy Markdown

👷 Deploy request for quirrel-docs pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit bd9ff32

The schema change alone (.min(1) -> .nonempty()) only narrows the internal
inferred type. The public hand-written EnqueueJobOptions interface still typed
retry as (number|string)[], so it (a) did not reflect the runtime non-empty
constraint users hit at runtime, and (b) was no longer assignable to the
schema-inferred non-empty tuple, breaking tsc at enqueue()/enqueueMany().

Tighten retry to [number|string, ...(number|string)[]] so the public type
rejects empty arrays at compile time, matching runtime validation. Closes quirrel-dev#1167.

Verified: tsc --noEmit clean; client retry + schema-checks tests pass.
@claygeo

claygeo commented Jun 19, 2026

Copy link
Copy Markdown
Author

Reviving this one. The bug in #1167 still reproduces on current main: the public EnqueueJobOptions.retry type accepts [], but the runtime schema rejects it ("Should have at least 1 items").

Verifying it surfaced something though, the original one-line schema change (.min(1) -> .nonempty()) isn't sufficient on its own:

  1. It only narrows the schema's inferred type. The type users actually consume is the hand-written EnqueueJobOptions interface (retry?: (number | string)[]), which was untouched, so an empty array still type-checks at the call site.
  2. In isolation it also breaks tsc: enqueue() / enqueueMany() pass that interface (a plain array) into payloadAndOptionsToBody, whose param is now the non-empty tuple, so (string | number)[] is no longer assignable (TS2345 at both call sites). There's no typecheck step in CI, so it slipped through.

So I pushed a follow-up commit (bd9ff32) tightening the interface to retry?: [number | string, ...(number | string)[]]. Now the public type rejects retry: [] at compile time, matching runtime, and tsc --noEmit is clean again. Ran the client retry and schema-checks suites locally, both green.

One intended consequence: this tightens a public type, so code that builds retry as a general number[]/string[] variable will now need a non-empty tuple or a literal. That's the behavior #1167 asks for.

(Forward-note: whenever zod moves to v4, .nonempty() no longer changes the inferred type, so the schema side would switch to z.tuple([...], rest). Not relevant for the pinned v3 here.)

Could a maintainer take another look when you get a chance? Happy to adjust.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EnqueueJobOptions retry type wrong

1 participant