Skip to content

chore(cli): HandlerArgv type#1933

Open
Tobbe wants to merge 1 commit into
mainfrom
tobbe-chore-js-to-ts-handler-argv-type
Open

chore(cli): HandlerArgv type#1933
Tobbe wants to merge 1 commit into
mainfrom
tobbe-chore-js-to-ts-handler-argv-type

Conversation

@Tobbe

@Tobbe Tobbe commented Jun 16, 2026

Copy link
Copy Markdown
Member

Use HandlerArgv in as many places as possible to DRY up the code

@netlify

netlify Bot commented Jun 16, 2026

Copy link
Copy Markdown

Deploy Preview for cedarjs canceled.

Name Link
🔨 Latest commit 94eb281
🔍 Latest deploy log https://app.netlify.com/projects/cedarjs/deploys/6a31ad004572380008b45696

@github-actions github-actions Bot added this to the chore milestone Jun 16, 2026
@greptile-apps

greptile-apps Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR removes the [key: string]: unknown index signature from HandlerArgv and introduces per-generator *Argv intersection types (e.g. CellArgv, ComponentArgv, FunctionFilesArgv) to replace the old catch-all ...argv spread pattern. It also consolidates the repetitive reduce/transformTSToJS loops into the shared transformTSToJSMap helper across several handlers.

  • HandlerArgv tightened — removing the index signature is the load-bearing change; all generator files() and handler() functions are updated to extend it with their own specific optional fields instead of using [key: string]: unknown.
  • Destroy genericcreateHandler in the destroy path becomes generic over TFilesArgs to accept the now-typed files functions, using an as unknown as TFilesArgs cast for the hardcoded internal arg set.
  • transformTSToJSMap adoptioncomponentHandler, functionHandler, jobHandler, and pageHandler all drop their bespoke async-reduce loops in favour of the shared helper.

Confidence Score: 4/5

Safe to merge — purely a typing refactor with no behavioural changes to any generator or destroy command.

All runtime behaviour is unchanged; the as unknown as TFilesArgs double-cast in the destroy helper and the redundant tests? field in FunctionFilesArgv are small type-hygiene issues with no current impact on generated output.

packages/cli/src/commands/destroy/handlerHelpers.ts — the generic bound on TFilesArgs could be narrowed to prevent future callers from accidentally passing a type with required fields that the hardcoded arg set cannot satisfy.

Important Files Changed

Filename Overview
packages/cli/src/commands/generate/yargsHandlerHelpers.ts Removes [key: string]: unknown index signature from HandlerArgv, making the shared type strict — the central change this PR builds on.
packages/cli/src/commands/destroy/handlerHelpers.ts Made FilesFunction and createHandler generic over TFilesArgs; passes fixed fields to filesFn with an as unknown as TFilesArgs double-cast that bypasses type safety.
packages/cli/src/commands/generate/function/functionHandler.ts Introduces FunctionFilesArgv using HandlerArgv base; redundantly re-declares tests?: boolean which is already in HandlerArgv.
packages/cli/src/commands/generate/cell/cellHandler.ts Replaces ...argv spread pattern with explicit destructured list and query fields via the new CellArgv = HandlerArgv & {...} type; clean refactor.
packages/cli/src/commands/generate/page/pageHandler.ts Extracts ParamVariants as a named type, adds it as Partial<ParamVariants> to PageArgv, and correctly removes raw path from the files() call since templates use the derived variant fields, not path directly.
packages/cli/src/commands/generate/component/componentHandler.ts Replaces manual reduce/transformTSToJS loop with transformTSToJSMap, and drops the unused javascript arg from the test-facing API via the new ComponentArgv type.
packages/cli/src/commands/generate/job/jobHandler.js Replaces inline async reduce loop with transformTSToJSMap; pure cleanup with no type changes (file remains .js).
packages/cli/src/commands/generate/component/tests/component.test.ts Removes the now-invalid javascript: true arg from test calls; correct cleanup since typescript defaults to false and the field was never consumed.
packages/cli/src/commands/generate/layout/tests/layout.test.ts Same cleanup as component tests — removes unused javascript: true from two test call-sites.

Reviews (1): Last reviewed commit: "chore(cli): HandlerArgv type" | Re-trigger Greptile

Comment on lines +13 to +16
type FunctionFilesArgv = HandlerArgv & {
typescript?: boolean
tests?: boolean
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 tests is already declared as tests?: boolean in HandlerArgv, so re-declaring it in the intersection type is redundant.

Suggested change
type FunctionFilesArgv = HandlerArgv & {
typescript?: boolean
tests?: boolean
}
type FunctionFilesArgv = HandlerArgv & {
typescript?: boolean
}

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment on lines 29 to +38
{
title: `Destroying ${componentName} files...`,
task: async () => {
const f = await filesFn({ name, stories: true, tests: true })
// The destroy flow always passes the same fixed set of fields to
// the generator's `files` function. We assert to TFilesArgs here
// because the caller is generic over what those extra fields are.
const f = await filesFn({
name,
stories: true,
tests: true,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 as unknown as TFilesArgs bypasses the generic bound

The double-cast hides any mismatch between the fixed { name, stories: true, tests: true } object and what TFilesArgs actually requires. If a future caller binds TFilesArgs to a type that has a required non-optional field beyond name, the compiler will silently accept the cast while the runtime will receive undefined for that field. Constraining the type parameter — e.g. TFilesArgs extends { name: string; stories?: boolean; tests?: boolean } — would surface exactly those cases at the call site without needing the double-cast.

@nx-cloud

nx-cloud Bot commented Jun 16, 2026

Copy link
Copy Markdown

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit 94eb281

Command Status Duration Result
nx run-many -t build:pack --exclude create-ceda... ✅ Succeeded 2s View ↗
nx run-many -t build ✅ Succeeded <1s View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-06-16 20:22:03 UTC

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