refactor: convert claycli to TypeScript (Phase 4)#238
Open
tkstang wants to merge 38 commits intoyolo-updatefrom
Open
refactor: convert claycli to TypeScript (Phase 4)#238tkstang wants to merge 38 commits intoyolo-updatefrom
tkstang wants to merge 38 commits intoyolo-updatefrom
Conversation
Add typescript, @types/node, ts-jest, and typescript-eslint as devDependencies. Create tsconfig.json with strict settings and noEmit for type checking. Configure Jest transform for .ts files via ts-jest. Add TypeScript-aware ESLint config block for .ts files with @typescript-eslint/no-unused-vars replacing the base rule.
Convert four leaf modules (no internal dependencies) from JS to TS: - lib/types.ts: readonly string array of Clay data types - lib/deep-reduce.ts: recursive component tree reducer with typed fn callback - lib/config-file-helpers.ts: config file reader with typed ConfigFile - lib/composer.ts: component normalize/denormalize with Bootstrap/ComponentRef interfaces Add @types/lodash for lodash type definitions. All modules use typed function signatures while keeping require() for untyped external deps.
Convert utility modules with typed function signatures: - lib/prefixes.ts: prefix add/remove with typed dispatch and string params - lib/compilation-helpers.ts: bucket, time, watcher with BrowserslistConfig - lib/formatting.ts: toDispatch/toBootstrap with Dispatch, Page, User interfaces - lib/reporters/*.ts: all 5 reporter modules with Action/Summary interfaces Uses import from 'lodash' for typed lodash calls, require() for untyped external deps (chalk, clay-log, terminal-logger, etc).
Convert rest, config, lint, export, import modules from JS to TS with full type annotations. Add caughtErrorsIgnorePattern to TS eslint rule for underscore-prefixed catch variables.
Rename all .js files in lib/cmd/compile/ and lib/cmd/pack/ to .ts
(except _client-init.js which is a client-side script read by gulp).
Add explicit type annotations for function parameters and callback
args. Install @types/jest for test file type checking. Update
eslint.config.js browser globals glob for renamed file. Add export {}
to test files so TypeScript treats them as modules.
Rename cli/*.js to .ts (except cli/index.js which is the bin entry point with shebang). Convert module.exports to export =, remove 'use strict', add type annotations to function params and callbacks. Keep cli/index.js and index.js as .js — they are package entry points that need a build step (p04-t07) before they can be converted.
Add tsconfig.build.json for compilation to dist/, update package.json entry points (main, types, bin) to point to dist/, add build/type-check scripts, and add dist/ to .gitignore. The published package now ships compiled JS with declaration files and source maps.
Document TypeScript conventions (export =, const require, tsconfig split), update tech stack (TypeScript 5, ts-jest, typescript-eslint), update file extensions (.ts), add build/type-check to commands and definition of done.
…kage.json copy to build Integration test revealed two issues when running from dist/: 1. resolveLoader only had 3-level __dirname traversal (works from source but not dist/lib/cmd/compile/). Added 4-level path for dist/ execution. 2. cli/index.ts requires ../package.json which resolves to dist/package.json when running from dist/. Added cp package.json dist/ to build script. Integration checkpoint 3 passes: 625 files compiled, 4571 JS outputs, 4046 registry entries (matches checkpoints 1 and 2).
All 5 phases complete. 3 integration checkpoints passed. Awaiting final review before PR.
Final review: 0 critical, 3 important, 5 medium, 6 minor. Converted 8 findings to fix tasks (p04-t10 through p04-t17). Deferred 2 with rationale: Highland retention (M1), babel-plugin-lodash warning (m4). 4 minor findings are non-issues requiring no action.
Remove dependency-tree, exports-loader, and imports-loader — none are imported anywhere in the codebase.
Replace any types with proper TypeScript types for the hard API contract with nymag/sites. Add GetDependenciesOptions interface, type all public and internal functions with string/boolean/Record types.
Migrate 4 occurrences of the deprecated url.parse() API to the modern WHATWG URL API (new URL()). Remove unused nodeUrl imports from both files.
…ions type Add FetchOptions interface extending RequestInit with optional agent property (supported by Node's undici fetch). Remove all 5 'as RequestInit' assertions from callers.
Remove setup-jest.js from both include and exclude arrays in the build config. It was confusing to have it in both. The base tsconfig.json retains it for type-checking; the build config doesn't need it.
path-browserify was a leftover from the Browserify era. Webpack 5 uses resolve.fallback with `path: false`, so the polyfill is never loaded.
All 8 review fix tasks (p04-t10 through p04-t17) completed. 48/48 tasks done. Reviews table updated to fixes_completed. Awaiting final re-review before PR.
Re-review passed with 0 findings. All 48 tasks done. Updated plan.md Reviews table to passed, state.md to complete, implementation.md Final Summary with review fixes and deferred items.
The p03-t10 stream detection fix was lost when resolving rebase conflicts between the p03 .js fixes and the p04 .ts conversion. Re-apply the check.
When rebasing typescript-conversion onto yolo-update, taking --theirs for OAT artifacts dropped the p03 review entries. Restore: - plan.md: p03-t09/t10/t11 task definitions, p03 review row, updated totals (51 tasks) - implementation.md: review notes, fix task entries, progress table (11/11 p03) - state.md: updated totals and last commit
…ndling p04 review found 1 Important finding: WHATWG URL migration breaks schemeless CLI inputs (domain.com/...). Added p04-t18 to fix urlToUri/getExt with fallback base URL parsing.
Add safeParseUrl helper that prepends http:// for schemeless inputs like domain.com/_pages/foo.html, restoring the behavior that url.parse() provided before the WHATWG URL migration. Add 4 regression tests.
All p04 review fix tasks implemented. 52/52 tasks complete. Awaiting p03/p04 re-reviews and final review before PR.
Restored mapConcurrent usage in export.ts, import.ts, lint.ts, and cli/lint.ts that was lost during TypeScript conversion. Thread concurrency through options objects to comply with max-params rule.
Add null guard for destFileStats in the extra-file comparison path. Prevents TypeError on first-run builds when dest directory is missing and options.extra is set.
…to fixes_completed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR: Progress — claycli modernization (Phase 4: TypeScript Conversion)
What
Converted the entire claycli codebase from JavaScript to TypeScript with strict mode. All source files are now
.ts(exceptsetup-jest.jsandeslint.config.js). TypeScript compiles to CommonJS viatsc, with declarations and source maps. The npm package ships compiled JS fromdist/. Review fixes addressed deprecated APIs, unused dependencies, type safety gaps, and build config cleanup.Why
TypeScript conversion completes the modernization plan by adding static type safety, enabling IDE-driven refactoring, and producing
.d.tsdeclarations for downstream consumers. This builds on the Node 20+ / Webpack 5 / async-await foundation established in Phases 0–3.Scope
.oat/projects/shared/claycli-modernizationyolo-update(Phases 0–3)Base tasks (p04-t01 through p04-t09)
typescript,ts-jest,typescript-eslint, stricttsconfig.jsontsconfig.build.json, entry points →dist/,npm run build/npm run type-checkReview fix tasks (p04-t10 through p04-t18)
new Buffer()withBuffer.from()(5 instances)nodeUrl.parse()withnew URL()(4 instances)FetchOptionsinterface to eliminateas RequestInitassertionsCommits
76f27ddchore(p04-t01): set up TypeScript infrastructurefaf498arefactor(p04-t02): convert leaf modules to TypeScripta781064refactor(p04-t03): convert utility modules to TypeScriptf1e3d9brefactor(p04-t04): convert core modules to TypeScript7edc0darefactor(p04-t05): convert compile and pack modules to TypeScriptcff9783refactor(p04-t06): convert CLI entry points to TypeScript66b35e6chore(p04-t07): configure TypeScript build for npm publishingf60db1edocs(p04-t08): update AGENTS.md for TypeScript codebase77d372ffix(p04-t09): fix resolveLoader paths for dist/ execution6918c38refactor(p04-t10): convert cli/compile files to TypeScript8896d86fix(p04-t11): replace deprecated new Buffer() with Buffer.from()0040032chore(p04-t12): remove unused production dependencies7e9c825refactor(p04-t13): add proper types to getDependencies API contractf4cb5e4fix(p04-t14): replace deprecated nodeUrl.parse with new URL()a2013d7fix(p04-t15): replace RequestInit type assertion with proper FetchOptions type5f25e00fix(p04-t16): clean up tsconfig.build.json include/exclude1dc132cchore(p04-t17): remove unused path-browserify dependencybbf4babfix(p04-t18): handle schemeless URLs in prefixes urlToUri/getExt5ad2b0bfix: restore stream detection in import.ts after rebaseValidation
tsc --noEmit(strict mode) — cleantsc -p tsconfig.build.json— 191 files in dist/fixes_completed(re-review pending)References