diff --git a/.github/workflows/jsweep.lock.yml b/.github/workflows/jsweep.lock.yml index a6e834141a..87fb345b0f 100644 --- a/.github/workflows/jsweep.lock.yml +++ b/.github/workflows/jsweep.lock.yml @@ -19,7 +19,7 @@ # gh aw compile # For more information: https://github.com/githubnext/gh-aw/blob/main/.github/aw/github-agentic-workflows.md # -# Daily JavaScript unbloater that cleans one .cjs file per day using modern JavaScript patterns +# Daily JavaScript unbloater that cleans one .cjs file per day, prioritizing files with @ts-nocheck to enable type checking name: "jsweep - JavaScript Unbloater" "on": @@ -558,7 +558,8 @@ jobs: - Files in `/home/runner/work/gh-aw/gh-aw/actions/setup/js/*.cjs` - Exclude test files (`*.test.cjs`) - Exclude files you've already cleaned (stored in cache-memory as `cleaned_files` array) - - Pick the **one file** with the earliest modification timestamp that hasn't been cleaned + - **Priority 1**: Pick files with `@ts-nocheck` or `// @ts-nocheck` comments (these need type checking enabled) + - **Priority 2**: If no uncleaned files with `@ts-nocheck` remain, pick the **one file** with the earliest modification timestamp that hasn't been cleaned If no uncleaned files remain, start over with the oldest cleaned file. @@ -566,6 +567,7 @@ jobs: Before making changes to the file: - Determine the execution context (github-script vs Node.js) + - **Check if the file has `@ts-nocheck` comment** - if so, your goal is to remove it and fix type errors - Identify code smells: unnecessary try/catch, verbose patterns, missing modern syntax - Check if the file has a corresponding test file - Read the test file to understand expected behavior @@ -574,6 +576,44 @@ jobs: Apply these principles to the file: + **Remove `@ts-nocheck` and Fix Type Errors (High Priority):** + ```javascript + // ❌ BEFORE: Type checking disabled + // @ts-nocheck - Type checking disabled due to complex type errors requiring refactoring + /// + + async function processData(data) { + return data.items.map(item => item.value); // Type errors ignored + } + + // ✅ AFTER: Type checking enabled with proper types + // @ts-check + /// + + /** + * Process data items + * @param {{ items: Array<{ value: string }> }} data - Input data + * @returns {Array} Processed values + */ + async function processData(data) { + return data.items.map(item => item.value); + } + ``` + + **Steps to remove `@ts-nocheck`:** + 1. Remove the `@ts-nocheck` comment from the file + 2. Replace it with `@ts-check` to enable type checking + 3. Run `npm run typecheck` to see type errors + 4. Fix type errors by: + - Adding JSDoc type annotations for functions and parameters + - Adding proper type declarations for variables + - Fixing incorrect type usage + - Adding proper null checks where needed + 5. Re-run `npm run typecheck` until all errors are resolved + 6. The file must pass type checking before creating the PR + + Apply these principles to the file: + **Remove Unnecessary Try/Catch:** ```javascript // ❌ BEFORE: Exception not handled with control flow @@ -688,14 +728,18 @@ jobs: ## Important Constraints + - **PRIORITIZE files with `@ts-nocheck`** - These files need type checking enabled. Remove `@ts-nocheck`, add proper type annotations, and fix all type errors. - **DO NOT change logic** - only make the code cleaner and more maintainable - **Always add or improve tests** - the file must have comprehensive test coverage with at least 5-10 test cases - **Always run tests** after changes to verify they pass: `npm run test:js` - **Always run TypeScript typecheck** before creating the PR to ensure type safety: `npm run typecheck` + - If the file had `@ts-nocheck`, it MUST pass typecheck after removing it - **Always run prettier** to format the code consistently: `npm run format:cjs` - **Preserve all functionality** - ensure the file works exactly as before - **One file per run** - focus on quality over quantity - - **Document your changes** in the PR description, including test improvements + - **Document your changes** in the PR description, including: + - Whether `@ts-nocheck` was removed and type errors fixed + - Test improvements (number of tests added, coverage improvements) ## Current Repository Context @@ -1242,7 +1286,7 @@ jobs: uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 env: WORKFLOW_NAME: "jsweep - JavaScript Unbloater" - WORKFLOW_DESCRIPTION: "Daily JavaScript unbloater that cleans one .cjs file per day using modern JavaScript patterns" + WORKFLOW_DESCRIPTION: "Daily JavaScript unbloater that cleans one .cjs file per day, prioritizing files with @ts-nocheck to enable type checking" with: script: | const fs = require('fs'); diff --git a/.github/workflows/jsweep.md b/.github/workflows/jsweep.md index f88e0b6ba8..b76644646e 100644 --- a/.github/workflows/jsweep.md +++ b/.github/workflows/jsweep.md @@ -1,5 +1,5 @@ --- -description: Daily JavaScript unbloater that cleans one .cjs file per day using modern JavaScript patterns +description: Daily JavaScript unbloater that cleans one .cjs file per day, prioritizing files with @ts-nocheck to enable type checking on: schedule: daily workflow_dispatch: @@ -57,7 +57,8 @@ Use cache-memory to track which files you've already cleaned. Look for: - Files in `/home/runner/work/gh-aw/gh-aw/actions/setup/js/*.cjs` - Exclude test files (`*.test.cjs`) - Exclude files you've already cleaned (stored in cache-memory as `cleaned_files` array) -- Pick the **one file** with the earliest modification timestamp that hasn't been cleaned +- **Priority 1**: Pick files with `@ts-nocheck` or `// @ts-nocheck` comments (these need type checking enabled) +- **Priority 2**: If no uncleaned files with `@ts-nocheck` remain, pick the **one file** with the earliest modification timestamp that hasn't been cleaned If no uncleaned files remain, start over with the oldest cleaned file. @@ -65,6 +66,7 @@ If no uncleaned files remain, start over with the oldest cleaned file. Before making changes to the file: - Determine the execution context (github-script vs Node.js) +- **Check if the file has `@ts-nocheck` comment** - if so, your goal is to remove it and fix type errors - Identify code smells: unnecessary try/catch, verbose patterns, missing modern syntax - Check if the file has a corresponding test file - Read the test file to understand expected behavior @@ -73,6 +75,44 @@ Before making changes to the file: Apply these principles to the file: +**Remove `@ts-nocheck` and Fix Type Errors (High Priority):** +```javascript +// ❌ BEFORE: Type checking disabled +// @ts-nocheck - Type checking disabled due to complex type errors requiring refactoring +/// + +async function processData(data) { + return data.items.map(item => item.value); // Type errors ignored +} + +// ✅ AFTER: Type checking enabled with proper types +// @ts-check +/// + +/** + * Process data items + * @param {{ items: Array<{ value: string }> }} data - Input data + * @returns {Array} Processed values + */ +async function processData(data) { + return data.items.map(item => item.value); +} +``` + +**Steps to remove `@ts-nocheck`:** +1. Remove the `@ts-nocheck` comment from the file +2. Replace it with `@ts-check` to enable type checking +3. Run `npm run typecheck` to see type errors +4. Fix type errors by: + - Adding JSDoc type annotations for functions and parameters + - Adding proper type declarations for variables + - Fixing incorrect type usage + - Adding proper null checks where needed +5. Re-run `npm run typecheck` until all errors are resolved +6. The file must pass type checking before creating the PR + +Apply these principles to the file: + **Remove Unnecessary Try/Catch:** ```javascript // ❌ BEFORE: Exception not handled with control flow @@ -187,14 +227,18 @@ After cleaning the file, adding/improving tests, and verifying all tests, TypeSc ## Important Constraints +- **PRIORITIZE files with `@ts-nocheck`** - These files need type checking enabled. Remove `@ts-nocheck`, add proper type annotations, and fix all type errors. - **DO NOT change logic** - only make the code cleaner and more maintainable - **Always add or improve tests** - the file must have comprehensive test coverage with at least 5-10 test cases - **Always run tests** after changes to verify they pass: `npm run test:js` - **Always run TypeScript typecheck** before creating the PR to ensure type safety: `npm run typecheck` + - If the file had `@ts-nocheck`, it MUST pass typecheck after removing it - **Always run prettier** to format the code consistently: `npm run format:cjs` - **Preserve all functionality** - ensure the file works exactly as before - **One file per run** - focus on quality over quantity -- **Document your changes** in the PR description, including test improvements +- **Document your changes** in the PR description, including: + - Whether `@ts-nocheck` was removed and type errors fixed + - Test improvements (number of tests added, coverage improvements) ## Current Repository Context diff --git a/pkg/workflow/jsweep_workflow_test.go b/pkg/workflow/jsweep_workflow_test.go index 1813dc9955..392fa169cf 100644 --- a/pkg/workflow/jsweep_workflow_test.go +++ b/pkg/workflow/jsweep_workflow_test.go @@ -27,7 +27,8 @@ func TestJSweepWorkflowConfiguration(t *testing.T) { if strings.Contains(mdContent, "three .cjs files per day") { t.Error("jsweep workflow should not process three files") } - if !strings.Contains(mdContent, "Pick the **one file**") { + // Check for "one file" in either Priority 1 or Priority 2 + if !strings.Contains(mdContent, "one file") { t.Error("jsweep workflow should pick one file") } if strings.Contains(mdContent, "Pick the **three files**") { @@ -104,7 +105,33 @@ func TestJSweepWorkflowConfiguration(t *testing.T) { } }) - // Test 8: Verify the workflow has a valid lock file + // Test 8: Verify the workflow prioritizes files with @ts-nocheck + t.Run("PrioritizesTsNocheck", func(t *testing.T) { + if !strings.Contains(mdContent, "Priority 1") { + t.Error("jsweep workflow should have Priority 1 for file selection") + } + if !strings.Contains(mdContent, "@ts-nocheck") { + t.Error("jsweep workflow should mention @ts-nocheck") + } + if !strings.Contains(mdContent, "these need type checking enabled") { + t.Error("jsweep workflow should explain why @ts-nocheck files are prioritized") + } + }) + + // Test 9: Verify the workflow has instructions to remove @ts-nocheck + t.Run("RemovesTsNocheck", func(t *testing.T) { + if !strings.Contains(mdContent, "Remove `@ts-nocheck`") { + t.Error("jsweep workflow should have instructions to remove @ts-nocheck") + } + if !strings.Contains(mdContent, "Replace it with `@ts-check`") { + t.Error("jsweep workflow should instruct replacing @ts-nocheck with @ts-check") + } + if !strings.Contains(mdContent, "Fix type errors") { + t.Error("jsweep workflow should mention fixing type errors") + } + }) + + // Test 10: Verify the workflow has a valid lock file t.Run("HasValidLockFile", func(t *testing.T) { lockPath := filepath.Join("..", "..", ".github", "workflows", "jsweep.lock.yml") _, err := os.Stat(lockPath) @@ -149,4 +176,14 @@ func TestJSweepWorkflowLockFile(t *testing.T) { t.Error("Compiled jsweep workflow should include prettier formatting") } }) + + // Test 4: Verify @ts-nocheck prioritization is in the compiled workflow + t.Run("CompiledTsNocheckPrioritization", func(t *testing.T) { + if !strings.Contains(lockContent, "Priority 1") { + t.Error("Compiled jsweep workflow should prioritize files with @ts-nocheck") + } + if !strings.Contains(lockContent, "@ts-nocheck") { + t.Error("Compiled jsweep workflow should mention @ts-nocheck") + } + }) }