Conversation
There was a problem hiding this comment.
Pull request overview
This PR completes the TypeScript migration by converting the remaining JavaScript files to TypeScript, reorganizing exports to be inline with their definitions, and updating the package configuration to properly reference the compiled output.
Key Changes:
- Converted core library files (
subprocess.js,exec.js,index.js) and rootindex.jsto TypeScript - Refactored exports from centralized bottom-of-file declarations to inline exports
- Updated package.json to point main entry to
build/index.jsand added types declaration
Reviewed changes
Copilot reviewed 11 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Removed checkJs option and added index.ts to includes array for proper TypeScript compilation |
| package.json | Updated main entry point to build/index.js, added types field, and simplified files array to include build directory |
| index.js → index.ts | Converted to TypeScript with simple re-export from lib |
| lib/index.js → lib/index.ts | Converted to TypeScript with inline named exports |
| lib/subprocess.js → lib/subprocess.ts | Converted to TypeScript with full type annotations, generic types, and proper event handling types |
| lib/exec.js → lib/exec.ts | Converted to TypeScript with comprehensive type definitions for options and return values |
| lib/helpers.ts | Updated imports to use node: protocol and refined JSDoc to standard TypeScript documentation |
| lib/circular-buffer.ts | Minor formatting fix (added closing brace) |
| test/exec-specs.ts | Added type assertions to stdout/stderr for type safety in tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Updated ✅ |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 13 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * | ||
| * @template TSubProcessOptions - Options type extending SubProcessOptions | ||
| * | ||
| * @fires SubProcess#output - Emitted when stdout or stderr receives data |
lib/index.ts
Outdated
| export {spawn} from 'node:child_process'; | ||
| export {SubProcess} from './subprocess'; | ||
| export {exec} from './exec'; | ||
| export type * from './types'; |
There was a problem hiding this comment.
please double check if we need to export all types. I prefer to explicitly list these that need to be public as changing some in the future might create breaking behaviors
There was a problem hiding this comment.
Sure, let me search types in our repo and expose only used ones
There was a problem hiding this comment.
Hm, I see TeenProcessExecResult but the type is not in this class so long. Based on the usage, I assume the type is ExecResult right now. I'll update ExecResult to TeenProcessExecResult for other modules
lib/subprocess.ts
Outdated
| private cmd: string; | ||
| private opts: TSubProcessOptions; | ||
| private expectingExit: boolean; | ||
| private rep: string; |
There was a problem hiding this comment.
it also makes sense to add readonly to these that are not supposed to be mutated during class lifecycle
lib/subprocess.ts
Outdated
| async start( | ||
| startDetector: StartDetector<TSubProcessOptions> | number | boolean | null = null, | ||
| timeoutMs: number | boolean | null = null, | ||
| detach = false, |
## [4.0.0](v3.0.6...v4.0.0) (2025-12-20) ### ⚠ BREAKING CHANGES * `TeenProcessExecOptions` type respects `SpawnOptions` in `node:child_process` of `timeout` type * Remove `handleLastLines` from SubProcess by following the TODO * Minimize exposed properties in `SubProcess` ### Features * convert remaining js to ts ([#462](#462)) ([a228fd8](a228fd8))
|
🎉 This PR is included in version 4.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
Oh... some typescript uses proper ExecError so let me keep ExecError instead of TeenProcessExecResult by following existing code. Update quickly |
exportfrom bottom to each definitionpackage.jsonto refer to proper index in buildBREAKING CHANGE:
TeenProcessExecOptionstype respectsSpawnOptionsinnode:child_processoftimeouttypeBREAKING CHANGE: Remove
handleLastLinesform SubProcess by following the TODOBREAKING CHANGE: Minimize exposed properties in
SubProcess