Skip to content

Fix Workflow interface to use array types instead of single-element tuples#187

Open
sohumt123 wants to merge 1 commit into
warpdotdev:mainfrom
sohumt123:fix/workflow-interface-array-types
Open

Fix Workflow interface to use array types instead of single-element tuples#187
sohumt123 wants to merge 1 commit into
warpdotdev:mainfrom
sohumt123:fix/workflow-interface-array-types

Conversation

@sohumt123

Copy link
Copy Markdown

Discord username (optional)

N/A

Description of changes

This is a type-only fix to the hand-written TypeScript source that backs the published warp-workflows npm package (build_ts/index.ts), plus a type-level regression test.

Problem

The exported Workflow interface declares three of its fields as single-element tuple types instead of arrays:

tags?: [string];
arguments?: [Argument];
shells?: [Shell];

A tuple type of length one accepts exactly one element. Under the package's strict compiler settings this means any workflow with two or more tags, arguments, or shells fails to type-check against the shipped dist/index.d.ts — and so do the repo's own documented examples:

  • FORMAT.md documents a for {{variable}} in {{sequence}}; do {{command}}; done command with three arguments, and a two-tag list tags: ["git", "GitHub"].
  • README.md's example workflow uses shells: [], which a one-tuple rejects for having too few elements.
  • 108 of the 332 specs currently shipped under specs/ have two or more arguments.

So the published types contradict both the documentation and the majority of the real specs. Downstream consumers of warp-workflows who build a Workflow object with more than one tag/argument/shell get spurious TS2322 errors.

Fix

Change the three fields to array types:

tags?: string[];
arguments?: Argument[];
shells?: Shell[];

These are the only tuple-typed fields in the interface set (Argument has scalar fields only, Shell is a const enum, WorkflowSlug is a string alias), so no other declarations need touching.

This is a declaration-only change. Runtime is unaffected: index.ts casts the yaml-loader output with as Workflow, and dist/index.d.ts is generated from this source (it regenerates with the corrected array types on the next build).

I also added a small type-level regression test so this can't silently regress:

  • build_ts/type-tests/workflow-types.test-d.ts — constructs two Workflow literals: one mirroring FORMAT.md's three-argument / two-tag / two-shell example, and one mirroring the README's single-tag / shells: [] example.
  • build_ts/tsconfig.typetest.json — extends tsconfig.json with noEmit and includes the fixture.
  • A check-types npm script (tsc -p tsconfig.typetest.json).
  • An exclude entry in tsconfig.json (node_modules, dist, type-tests) so the fixture is never emitted into dist or the published bundle. I verified the emitted dist/ is unchanged apart from the corrected index.d.ts.

I intentionally did not wire check-types into the build script so the publish pipeline stays untouched — happy to add it (or a CI step) as a follow-up if you'd prefer it gated.

Testing

Using the repo's pinned toolchain (typescript@4.5.5, from build_ts/package-lock.json):

Baseline sanity on the existing sources (unchanged behavior):

$ npx tsc --noEmit
$ echo $?
0

The regression fixture before the fix (against the original tuple types):

$ npx tsc -p tsconfig.typetest.json
type-tests/workflow-types.test-d.ts(20,19): error TS2322: Type 'string' is not assignable to type 'undefined'.
type-tests/workflow-types.test-d.ts(24,5): error TS2322: Type '{ name: string; }' is not assignable to type 'undefined'.
type-tests/workflow-types.test-d.ts(25,5): error TS2322: Type '{ name: string; }' is not assignable to type 'undefined'.
type-tests/workflow-types.test-d.ts(27,24): error TS2322: Type 'Shell.Zsh' is not assignable to type 'undefined'.
type-tests/workflow-types.test-d.ts(40,3): error TS2322: Type '[]' is not assignable to type '[Shell]'.
  Source has 0 element(s) but target requires 1.
$ echo $?
2

The same command after the fix, and the full build:

$ npm run check-types    # tsc -p tsconfig.typetest.json
$ echo $?
0

$ npm run build          # tsc && webpack
webpack 5.107.2 compiled successfully
$ echo $?
0

dist/index.d.ts after the build now reads:

tags?: string[];
arguments?: Argument[];
shells?: Shell[];

and dist/ contains only index.js, index.d.ts, index.js.map, and warp-workflows.js (the fixture is excluded). crate-ci/typos passes clean on the changed files.

Notes

  • PR CI does not compile TypeScript (the ci.yaml job only runs typos + cargo), so this fixture can't fail/pass in the PR checks — the TS build runs in publish.yaml after merge. The before/after tsc output above is the evidence.
  • Since the change corrects an overly-narrow type, a consumer compiling with noUncheckedIndexedAccess will see e.g. workflow.tags[0] change from string to string | undefined. That is the correct typing for an array, but flagging it in case it surfaces anything downstream.

…uples

The exported `Workflow` interface declared `tags`, `arguments`, and `shells`
as single-element tuple types (`[string]`, `[Argument]`, `[Shell]`) rather
than arrays. A tuple type of length one only accepts exactly one element, so
under `strict` type-checking any workflow with two or more tags, arguments,
or shells fails to type-check against the published `warp-workflows` types --
including the repo's own documented examples (FORMAT.md's three-argument
for-loop and two-tag list, and the README example's `shells: []`, which a
one-tuple rejects for having too few elements). 108 of the 332 shipped specs
have two or more arguments.

Change the three fields to array types (`string[]`, `Argument[]`, `Shell[]`).
This is a declaration-only change; runtime is unaffected because index.ts casts
the yaml-loader output with `as Workflow`. dist/index.d.ts regenerates from this
source with the corrected array types.

Add a type-level regression test (build_ts/type-tests/workflow-types.test-d.ts)
that constructs Workflow literals mirroring the documented multi-value and
empty-shells examples, compiled via a new tsconfig.typetest.json through the
`check-types` script. It fails to compile before this change and passes after.
An `exclude` entry keeps the fixture out of the emitted dist bundle.
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.

1 participant