Skip to content

Replace any-casts in command-registry with a typed command lookup - #8439

Draft
alfonso-noriega wants to merge 1 commit into
mainfrom
typed-command-registry
Draft

Replace any-casts in command-registry with a typed command lookup#8439
alfonso-noriega wants to merge 1 commit into
mainfrom
typed-command-registry

Conversation

@alfonso-noriega

Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

packages/cli/src/command-registry.ts carried 7 eslint-disable @typescript-eslint/no-explicit-any suppressions: the lazy command loader returned Promise<any | undefined> and each external plugin branch indexed its command table through as any. The module also had no unit tests. Surfaced by the daily maintenance radar (suppression cleanup).

WHAT is this pull request doing?

  • Types the loader against @oclif/core's Command class (CommandClass = typeof Command), matching the LazyCommandLoader contract in cli-kit ((id: string) => Promise<typeof Command | undefined>).
  • Bridges the external plugin package tables (@shopify/cli-hydrogen, @oclif/plugin-commands, @oclif/plugin-plugins, @shopify/plugin-did-you-mean) through a single commandFromTable helper — one documented cast at the package boundary instead of four inline as any, since each package compiles against its own @oclif/core.
  • Adds command-registry.test.ts covering manifest misses and the full-package fallback path (real imports, no mocks).
  • Adds a @shopify/plugin-did-you-meansrc alias to the shared vitest config so the module under test resolves in vitest, same pattern as the existing @shopify/theme/@shopify/organizations aliases.

No runtime behavior change: the lookups and fallbacks are identical, only the types changed.

How to test your changes?

  • pnpm vitest run packages/cli/src/command-registry.test.ts (3 tests)
  • pnpm --filter @shopify/cli type-check && pnpm --filter @shopify/cli lint
  • Sanity: pnpm shopify commands and pnpm shopify plugins still resolve through the full-package fallback; any app/theme command still loads per-file.

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've considered analytics changes to measure impact
  • The change is user-facing — I've identified the correct bump type (patch for bug fixes · minor for new features · major for breaking changes) and added a changeset with pnpm changeset add

No changeset: internal refactor + tests, no user-visible change.

Type the lazy command loader against @oclif/core's Command class and
bridge the external plugin package tables through a single helper,
removing the per-branch eslint-disable/any casts. Add unit tests for
manifest misses and the full-package fallback path.

Assisted-By: devx/c4b514ef-dee6-425a-99d0-2f300e272eb6
@github-actions github-actions Bot added the no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users. label Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant