feat: merge Ready and EventEmitter #5
Conversation
WalkthroughThe pull request introduces several updates across the project's configuration and source code. The changes primarily focus on enhancing Node.js version support in CI workflows, updating dependency management, and improving the Changes
Sequence DiagramsequenceDiagram
participant User
participant ReadyEventEmitter
participant Ready
participant EventEmitter
User->>ReadyEventEmitter: Create instance
ReadyEventEmitter->>Ready: Initialize internal ready object
ReadyEventEmitter->>EventEmitter: Extend event handling
User->>ReadyEventEmitter: Call ready()
ReadyEventEmitter->>Ready: Delegate ready method
Ready-->>ReadyEventEmitter: Handle readiness
ReadyEventEmitter->>EventEmitter: Emit 'ready-event'
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/[email protected] |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
README.md (1)
55-63: Enhance ReadyEventEmitter documentationThe ReadyEventEmitter example could be more comprehensive to demonstrate:
- How it combines Ready and EventEmitter functionality
- Event handling patterns
- Common use cases
Consider expanding the example:
import { ReadyEventEmitter } from 'get-ready'; class MyClass extends ReadyEventEmitter { - // your handler here + async initialize() { + // Async initialization + await someAsyncOperation(); + + // Mark as ready when done + this.ready(true); + } } + +// Usage example +const instance = new MyClass(); + +// Listen for ready event +instance.on('ready', () => { + console.log('Instance is ready!'); +}); + +// Or use the promise-based API +await instance.ready();src/index.ts (3)
Line range hint
16-24: Consider adding JSDoc for method overloadsThe implementation looks good with proper type safety through method overloads. Consider adding JSDoc comments to document the overloads for better developer experience.
+/** + * Register a callback or return a promise that resolves when ready + * @overload + * @returns Promise that resolves when ready + */ ready(): Promise<void>; +/** + * Register a callback or set ready state + * @overload + * @param flagOrFunction - Callback function or ready state + */ ready(flagOrFunction: ReadyFunctionArg): void;
83-85: Improve type safety in mixin implementationThe current implementation uses
anytype which could be made more type-safe.-obj.ready = (flagOrFunction: any) => { +obj.ready = <T extends ReadyFunctionArg | undefined>(flagOrFunction?: T): T extends undefined ? Promise<void> : void => { return ready.ready(flagOrFunction); };
91-102: Add class documentation for ReadyEventEmitterThe implementation looks good, but would benefit from documentation explaining its purpose and usage.
+/** + * Combines EventEmitter functionality with Ready pattern + * Allows objects to both emit events and handle ready state + * @example + * class MyClass extends ReadyEventEmitter { + * constructor() { + * super(); + * this.ready(() => this.emit('ready')); + * } + * } + */ export class ReadyEventEmitter extends EventEmitter {test/index.test.ts (1)
138-150: Consider adding more test cases for ReadyEventClassWhile the basic functionality is tested, consider adding tests for:
- Error handling with events
- Multiple event listeners
- Event emission order
it('should emit error event', async () => { const someClass = new ReadyEventClass(); let error; someClass.on('error', (err) => { error = err; }); someClass.ready(new Error('test')); await setImmediate(); assert(error?.message === 'test'); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/nodejs.yml(1 hunks).github/workflows/release.yml(0 hunks)README.md(3 hunks)package.json(2 hunks)src/index.ts(5 hunks)test/index.test.ts(7 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/release.yml
🔇 Additional comments (10)
.github/workflows/nodejs.yml (2)
14-14: LGTM: Node.js 22 support added
Adding Node.js 22 support helps keep the package current with latest Node.js versions.
16-17: LGTM: CodeCov token configuration
Proper configuration of CODECOV_TOKEN secret enables accurate coverage reporting.
package.json (2)
66-68: LGTM: Proper dual CJS/ESM packaging configuration
The explicit configuration of types, main, and module fields alongside the exports field ensures proper module resolution in both CommonJS and ESM environments.
17-17: Verify compatibility with tshy major version update
The update from tshy ^1.2.2 to 3 is a major version bump that might include breaking changes.
✅ Verification successful
Let me try to verify the tshy dependency changes through package.json and any configuration files.
Let me try one more verification to check the actual package.json content and any TypeScript configuration that might be affected by tshy.
The tshy v3 update appears compatible with the project configuration
The project is properly configured for tshy v3 with:
- Correct tshy configuration in package.json using the new exports field format
- Proper TypeScript configuration with "module": "nodenext"
- Aligned dual package (CJS/ESM) configuration with main/module/types fields
- Compatible build script setup using "tshy && tshy-after"
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for breaking changes in tshy changelog
curl -s https://raw.githubusercontent.com/unjs/tshy/main/CHANGELOG.md | head -n 20
Length of output: 99
Script:
#!/bin/bash
# Check for tshy config files and package.json
echo "=== Package.json diff ==="
git diff origin/main -- package.json
echo -e "\n=== Find tshy config files ==="
fd -e json -e js tshy
fd -g "*.tshy.*"
echo -e "\n=== Check current tshy config if exists ==="
find . -maxdepth 1 -name "tshy.config.*" -exec cat {} \;
Length of output: 418
Script:
#!/bin/bash
# Check package.json and TypeScript config
echo "=== Current package.json ==="
cat package.json
echo -e "\n=== TypeScript configs ==="
fd -e json tsconfig
find . -maxdepth 1 -name "tsconfig*.json" -exec cat {} \;
Length of output: 2085
README.md (2)
7-7: LGTM: Added Node.js version badge
The Node.js version badge improves visibility of version compatibility.
95-97: LGTM: Modern contributor visualization
Using contrib.rocks provides a more visually appealing way to showcase contributors.
src/index.ts (2)
1-4: LGTM! Good practices in imports and type definitions
- Using
node:protocol for imports is a good practice for better Node.js compatibility - The
ReadyFunctionArgtype update aligns well with the new method overloads
Line range hint 32-57: LGTM! Well-implemented promise support
The promise implementation follows Node.js best practices with proper error handling and execution order using process.nextTick.
test/index.test.ts (2)
Line range hint 1-55: LGTM! Good test coverage for new functionality
The test implementations properly cover both callback and promise patterns, and the new ReadyEventClass provides a good example of ReadyEventEmitter usage.
155-156: LGTM! Comprehensive promise and error handling tests
The tests properly cover promise chains, async/await patterns, and error handling scenarios.
Also applies to: 183-204
[skip ci] ## [3.2.0](v3.1.0...v3.2.0) (2024-12-15) ### Features * merge Ready and EventEmitter ([#5](#5)) ([bb049ea](bb049ea))
Summary by CodeRabbit
New Features
ReadyEventEmitterclass for event-driven readiness handling.ReadyEventClassto manage 'ready-event' emissions.ReadyEventEmitter.Bug Fixes
Readyclass.Chores
package.jsonand removed unnecessary dependencies.