-
Notifications
You must be signed in to change notification settings - Fork 0
GFE Custom EventEmitter #187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Learn moreAll Green is an AI agent that automatically: ✅ Addresses code review comments ✅ Fixes failing CI checks ✅ Resolves merge conflicts |
|
@codex review |
1 similar comment
|
@codex review |
📝 WalkthroughWalkthroughA new EventEmitter utility has been introduced to GreatFrontEnd with comprehensive documentation and test coverage. The implementation provides observer/pub-sub pattern functionality, and progress trackers have been updated across README files to reflect the completion. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Claude finished @pertrai1's task in 1m 41s —— View job GreatFrontEnd EventEmitter Review ✅Excellent implementation! This is a clean, well-tested EventEmitter that correctly implements the observer pattern. The code demonstrates strong understanding of JavaScript fundamentals and includes comprehensive test coverage. 1. Correctness ✅Strengths:
Edge Cases Handled:
2. API Design & Usability ✅Excellent choices:
Minor improvements suggested:
3. Memory Management & PerformanceStrong points:
Optimization opportunities:
Time Complexity:
Space Complexity: O(E × L) where E = unique events, L = average listeners per event 4. Code Quality ✅JavaScript Best Practices:
Clean code:
5. Test Coverage 🌟Outstanding test suite! 367 lines of comprehensive tests covering:
The test comments (e.g., lines 250, 264, 282) demonstrate clear understanding of the expected behavior. 🎓 Learning SummaryPattern: Observer (Pub-Sub)This is a classic Observer Pattern implementation, also called Publish-Subscribe. It decouples event emitters from event listeners, allowing one-to-many communication. Key Insight 🔑The critical insight is using
This is a common pattern in event systems and is essential for robust implementations. JavaScript Concepts Demonstrated
Common Pitfalls Avoided ✅
Related Problems to Practice
Real-World Applications
Alternative Approaches
Trade-offs: Your array-based approach is simple, matches Node.js behavior (allows duplicates), and performs well for typical use cases. SummaryThis is a strong, production-ready implementation with excellent test coverage. The code is clean, follows best practices, and demonstrates solid understanding of JavaScript fundamentals and design patterns. Minor improvements suggested in inline comments are optional optimizations - the current implementation is already very good! Grade: A 🎉 |
|
To use Codex here, create a Codex account and connect to github. |
1 similar comment
|
To use Codex here, create a Codex account and connect to github. |
🧮 Solution AnalysisPR: GFE Custom EventEmitter 📊 Code Quality ResultsESLint Analysis
Issues by file:
🔍 Complexity Analysisevent-emitter📋 Analysis Results: 🔍 Automated Complexity Analysis
File: greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.ts Time ComplexityEstimated: O(1)
Space ComplexityEstimated: O(n) Data Structures Used
Code Metrics
Recommendations
event-emitter.test📋 Analysis Results: 🔍 Automated Complexity Analysis
File: greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.test.ts Time ComplexityEstimated: O(1)
Space ComplexityEstimated: O(1) Data Structures Used
Code Metrics
Recommendations
💡 Analysis Tips
📁 Analysis Files CreatedDetailed analysis files have been automatically generated and committed to 🎯 Next Steps
Analysis generated for PR files only |
✅ Test ResultsTests have been executed for this pull request. 📊 Coverage Summary
Check the workflow logs for detailed test output. |
|
🤖 Gemini Code Review (Model: Here's a detailed review of your 1. Pattern IdentificationThis solution implements the Observer Pattern (also known as the Publish-Subscribe pattern). In this pattern:
2. Complexity VerificationLet
3. Key InsightThe key insight for a robust
By creating 4. Edge Case AnalysisThe current solution handles most common edge cases correctly:
5. Learning Points
6. Code Quality
7. Alternative Approaches
Your chosen approach using |
|
🤖 Gemini Code Review (Model: Here's a review of the 1. Pattern IdentificationThis solution implements the Observer Pattern (also known as the Publish-Subscribe pattern).
The core idea is to establish a one-to-many dependency between objects so that when one object changes state, all its dependents are notified and updated automatically. In this case, when an event is "published" ( 2. Complexity VerificationTime Complexity
Space Complexity
3. Key InsightThe key insight for building a robust
4. Edge Case AnalysisThe current solution handles many edge cases well, but here are some specific considerations:
5. Learning Points
6. Code Quality
7. Alternative Approaches
|
| @@ -0,0 +1,50 @@ | |||
| export interface IEventEmitter { | |||
| on(eventName: string, listener: Function): IEventEmitter; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type Safety Improvement
Consider using a more specific type than Function for better type safety:
type EventListener = (...args: any[]) => void;This makes it clear that listeners don't return values (or their return values are ignored), which matches the actual behavior in emit() on line 45-46.
| #events; | ||
|
|
||
| constructor() { | ||
| this.#events = new Map(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Memory Consideration
The Map stores arrays of listeners indefinitely. Consider adding a method to clean up empty event arrays to prevent memory accumulation:
private cleanupEvent(eventName: string): void {
if (this.#events.has(eventName) && this.#events.get(eventName).length === 0) {
this.#events.delete(eventName);
}
}Then call this in off() after removing a listener. This prevents the Map from growing unbounded with event names that no longer have listeners.
| return this; | ||
| } | ||
| const listeners = this.#events.get(eventName); | ||
| const index = listeners.findIndex( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optimization Opportunity
findIndex() with a comparison function is unnecessary here. Since you're checking reference equality, use indexOf() directly:
const index = listeners.indexOf(listener);This is simpler, more performant, and achieves the same result.
| } | ||
| const listeners = this.#events.get(eventName).slice(); | ||
| listeners.forEach((listener: Function) => { | ||
| listener.apply(this, args); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this Context Consideration
Using listener.apply(this, args) binds this to the EventEmitter instance. While this matches Node.js behavior, it's worth considering whether this is the desired behavior for your use case.
Alternative approaches:
- Call directly (loses
thiscontext):listener(...args) - Bind at registration (lets users control context): Store bound functions in
on() - Current approach (EventEmitter as
this): Matches Node.js
The current implementation is correct for mimicking Node.js EventEmitter. Just be aware that listeners receive the EventEmitter as this, which might be unexpected for some users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a custom EventEmitter implementation to the GreatFrontEnd collection. The EventEmitter follows the observer/publish-subscribe pattern, similar to Node.js's EventEmitter, allowing registration of event listeners, removal of listeners, and event emission with arguments.
Changes:
- Implemented
EventEmitterclass withon(),off(), andemit()methods using TypeScript - Added comprehensive test suite with 367 lines covering edge cases, method chaining, concurrent modifications, and integration scenarios
- Updated progress trackers across README files (6/75 GFE problems, 11 total frontend problems)
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.ts |
Core EventEmitter implementation using Map for event storage and private fields for encapsulation |
greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.test.ts |
Comprehensive test suite covering all methods, edge cases, and integration scenarios |
greatfrontend/gfe-75/javascript-utilities/event-emitter/README.md |
Problem description with usage examples demonstrating the pub-sub pattern |
greatfrontend/gfe-75/javascript-utilities/README.md |
Added EventEmitter to the completed problems list |
greatfrontend/gfe-75/README.md |
Updated progress counter from 5 to 6 completed problems |
README.md |
Updated GreatFrontEnd problem count from 10 to 11 |
| } | ||
| const listeners = this.#events.get(eventName); | ||
| const index = listeners.findIndex( | ||
| (listenerIdx: Function) => listenerIdx === listener |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable name listenerIdx is misleading because it doesn't represent an index - it represents a listener function. Consider renaming it to something like listenerFn or l to better reflect its purpose.
| (listenerIdx: Function) => listenerIdx === listener | |
| (listenerFn: Function) => listenerFn === listener |
| } | ||
|
|
||
| export class EventEmitter implements IEventEmitter { | ||
| #events; |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The private field #events lacks a type annotation. While TypeScript can infer the type, explicit typing improves code clarity and maintainability. Consider adding the type annotation: #events: Map<string, Function[]>;
| #events; | |
| #events: Map<string, Function[]>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.test.ts:
- Around line 239-251: The test name is misleading and missing an assertion for
listener2: rename the test to reflect that errors from listeners propagate
(e.g., "should propagate errors from a listener and prevent subsequent
listeners") and keep the existing expect(() =>
emitter.emit('test')).toThrow('Listener error'); then add an assertion verifying
listener2 was not called (expect(listener2).not.toHaveBeenCalled()); reference
EventEmitter, emit, on, listener1 and listener2 to locate where to change.
🧹 Nitpick comments (4)
greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.ts (3)
8-8: Add explicit type annotation to the private field.Per coding guidelines, TypeScript should use explicit type annotations. The
#eventsfield lacks a type annotation.Suggested fix
- #events; + #events: Map<string, Function[]>;
14-20: Add explicit return type annotation toonmethod.Per coding guidelines, use explicit type annotations for function return types.
Suggested fix
- on(eventName: string, listener: Function) { + on(eventName: string, listener: Function): IEventEmitter {
22-35: Add explicit return type annotation tooffmethod.Per coding guidelines, use explicit type annotations for function return types.
Suggested fix
- off(eventName: string, listener: Function) { + off(eventName: string, listener: Function): IEventEmitter {greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.test.ts (1)
1-4: Consider adding edge case tests for event name validation.The test suite is comprehensive, but consider adding tests for edge cases around event names such as empty strings, special characters, or whitespace-only names to verify the implementation handles these gracefully. Based on learnings, edge case tests for empty inputs and boundary conditions are recommended.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.mdgreatfrontend/gfe-75/README.mdgreatfrontend/gfe-75/javascript-utilities/README.mdgreatfrontend/gfe-75/javascript-utilities/event-emitter/README.mdgreatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.test.tsgreatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.ts
🧰 Additional context used
📓 Path-based instructions (11)
**/README.md
📄 CodeRabbit inference engine (CLAUDE.md)
Include problem title and difficulty badge, link to the problem on the respective platform (LeetCode, GreatFrontEnd, etc.), problem description, examples and constraints
Files:
greatfrontend/gfe-75/javascript-utilities/event-emitter/README.mdREADME.mdgreatfrontend/gfe-75/javascript-utilities/README.mdgreatfrontend/gfe-75/README.md
README.md
📄 CodeRabbit inference engine (AGENTS.md)
Update README.md when adding new solutions
Files:
README.md
**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.ts: Use explicit type annotations for function parameters and return types
Follow strict TypeScript configuration (defined intsconfig.json)
Target ES2020 with CommonJS modules
Strict mode enabled with ES2020 target and CommonJS modules for TypeScript
**/*.ts: Use explicit type annotations for function parameters and return types in TypeScript solutions
Follow strict TypeScript configuration with ES2020 target and CommonJS modules
Avoidanytype unless absolutely necessary in TypeScript solutions
Use interfaces for object types in TypeScript solutions
Files:
greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.tsgreatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.test.ts
**/*.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,js}: Test complex algorithms with helper functions, data structure implementations, solutions with multiple edge cases, GreatFrontEnd API implementations, and reusable utility functions
Use descriptive variable names (single letters likei,j,kare acceptable for loop iterators and standard mathematical contexts)
Don't use deprecated JavaScript features
Files:
greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.tsgreatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.test.ts
**/*-*.{ts,js,md}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use kebab-case for file names matching problem names
Files:
greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.tsgreatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.test.ts
**/*.{js,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,ts}: Verify the solution correctly solves the problem for all test cases, including edge cases: empty arrays, single elements, negative numbers, null values
Document and verify time and space complexity using Big O notation, typically in code comments or docs/analysis/ files
Use clear, descriptive variable names (not just i, j, k unless in simple loops) and add comments for non-obvious logic or algorithm steps
Use const by default, let only when reassignment is needed; avoid var except for LeetCode solution function definitions
Use strict equality (===) over loose equality (==)
Use modern ES6+ syntax (arrow functions, destructuring, spread operators) where appropriate and prefer built-in methods (.map(), .filter(), .reduce())
**/*.{js,ts}: Useconstby default,letonly when reassignment is needed in JavaScript/TypeScript
Use strict equality (===) over loose equality (==) in JavaScript/TypeScript
Use descriptive variable names; single letters likei,j,kacceptable only for standard loop contexts
Prefer built-in array methods (.map(),.filter(),.reduce()) in JavaScript/TypeScript
Fix ESLint errors withnpm run quality:fixand ensure proper code formatting with Prettier
Files:
greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.tsgreatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.test.ts
**/greatfrontend/**/*.{js,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
**/greatfrontend/**/*.{js,ts}: Verify correct usage of browser APIs (DOM, Fetch, Storage, etc.), check for browser compatibility considerations, and validate event handling patterns with proper cleanup
Check for unnecessary re-renders or DOM manipulations, verify efficient data structure usage for UI updates, and look for debouncing/throttling where appropriate
Verify parameter validation, error handling, and consider edge cases specific to browser environments (empty inputs, invalid types)
Files:
greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.tsgreatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.test.ts
**/*.test.{ts,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
*.test.tsor*.test.jsnaming convention for test files
Files:
greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.test.ts
**/*.test.{js,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Vitest for testing complex solutions, data structures, and GreatFrontEnd implementations; ensure tests handle edge cases, async operations, and cleanup logic
Files:
greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.ts: Test all examples from problem description in test files
Add edge case tests: empty inputs, single elements, nulls, boundary conditions in test files
Use descriptive test names that explain what is being tested in test files
Group related tests usingdescribeblocks in test files
Include both positive and negative test cases in test files
Add test cases for new solutions and update tests when refactoring existing solutions
Files:
greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.test.ts
**/*-*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Test files should use kebab-case naming with
.test.tsextension:{problem-name}.test.ts
Files:
greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.test.ts
🧠 Learnings (7)
📚 Learning: 2025-12-30T13:41:58.622Z
Learnt from: CR
Repo: pertrai1/coding-challenges PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T13:41:58.622Z
Learning: Applies to leetcode/**/README.md : Keep README files up to date with problem counts and organization
Applied to files:
README.md
📚 Learning: 2025-12-30T13:41:24.644Z
Learnt from: CR
Repo: pertrai1/coding-challenges PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T13:41:24.644Z
Learning: Applies to **/README.md : Include problem title and difficulty badge, link to the problem on the respective platform (LeetCode, GreatFrontEnd, etc.), problem description, examples and constraints
Applied to files:
README.md
📚 Learning: 2025-12-30T13:41:58.622Z
Learnt from: CR
Repo: pertrai1/coding-challenges PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T13:41:58.622Z
Learning: Applies to **/*.test.ts : Add test cases for new solutions and update tests when refactoring existing solutions
Applied to files:
greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.test.ts
📚 Learning: 2025-12-30T13:41:58.622Z
Learnt from: CR
Repo: pertrai1/coding-challenges PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T13:41:58.622Z
Learning: Applies to **/*.test.ts : Add edge case tests: empty inputs, single elements, nulls, boundary conditions in test files
Applied to files:
greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.test.ts
📚 Learning: 2025-12-30T13:41:07.657Z
Learnt from: CR
Repo: pertrai1/coding-challenges PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-30T13:41:07.657Z
Learning: Applies to **/*.{ts,js} : Test complex algorithms with helper functions, data structure implementations, solutions with multiple edge cases, GreatFrontEnd API implementations, and reusable utility functions
Applied to files:
greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.test.tsgreatfrontend/gfe-75/README.md
📚 Learning: 2025-12-30T13:41:24.644Z
Learnt from: CR
Repo: pertrai1/coding-challenges PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-30T13:41:24.644Z
Learning: Applies to **/*.test.{js,ts} : Use Vitest for testing complex solutions, data structures, and GreatFrontEnd implementations; ensure tests handle edge cases, async operations, and cleanup logic
Applied to files:
greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.test.ts
📚 Learning: 2025-12-30T13:41:58.622Z
Learnt from: CR
Repo: pertrai1/coding-challenges PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-30T13:41:58.622Z
Learning: Applies to **/*.test.ts : Group related tests using `describe` blocks in test files
Applied to files:
greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.test.ts
🧬 Code graph analysis (1)
greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.test.ts (1)
greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.ts (2)
EventEmitter(7-50)IEventEmitter(1-5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Agent
- GitHub Check: review
🔇 Additional comments (9)
greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.ts (1)
37-49: Good defensive cloning of listeners array before iteration.The use of
.slice()on line 44 correctly prevents issues when listeners modify the listener list during emission (e.g., callingoffwithin a listener callback). This is a solid implementation detail.README.md (1)
10-10: Progress update looks correct.The GreatFrontEnd problem count increment to 11 aligns with the addition of the EventEmitter solution.
greatfrontend/gfe-75/javascript-utilities/README.md (1)
13-13: EventEmitter entry added correctly.The new entry follows the established format and links to the appropriate directory.
greatfrontend/gfe-75/javascript-utilities/event-emitter/README.md (1)
1-27: Well-structured documentation with clear examples.The README provides a concise explanation of the observer pattern, links to Node.js documentation for reference, and includes practical usage examples demonstrating
on,emit, andofffunctionality.greatfrontend/gfe-75/README.md (1)
7-7: Progress tracker and completed problems list updated correctly.The status increment to 6/75 and the new EventEmitter entry are consistent with the solution being added.
Also applies to: 17-17
greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.test.ts (4)
5-65: LGTM!The
on()test suite is comprehensive, covering basic registration, multiple listeners, per-event separation, method chaining, and non-idempotent duplicate registration. Test names are descriptive and assertions are appropriate.
67-144: LGTM!The
off()test suite thoroughly covers listener removal scenarios including partial removal of duplicate registrations, graceful handling of non-existent listeners/events, and event isolation. The tests correctly verify that only the first occurrence is removed when a listener is registered multiple times.
253-290: LGTM!These tests correctly verify the copy-on-emit behavior where listeners added or removed during emission don't affect the current emit cycle. The follow-up assertions confirm the changes take effect on subsequent emits.
293-366: LGTM!The integration tests effectively demonstrate real-world usage patterns including pub-sub messaging, chained event workflows, interface compliance, and basic stress testing with rapid successive events.
| it('should not fail if a listener throws an error', () => { | ||
| const emitter = new EventEmitter(); | ||
| const listener1 = vi.fn(() => { | ||
| throw new Error('Listener error'); | ||
| }); | ||
| const listener2 = vi.fn(); | ||
|
|
||
| emitter.on('test', listener1); | ||
| emitter.on('test', listener2); | ||
|
|
||
| expect(() => emitter.emit('test')).toThrow('Listener error'); | ||
| // Note: listener2 won't be called because listener1 throws | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test name is misleading and missing assertion for listener2.
The test is named "should not fail if a listener throws an error" but it actually expects the error to propagate (line 249). Additionally, the comment on line 250 states that listener2 won't be called, but there's no assertion to verify this behavior.
Consider renaming and adding the missing assertion:
Proposed fix
- it('should not fail if a listener throws an error', () => {
+ it('should propagate errors thrown by listeners', () => {
const emitter = new EventEmitter();
const listener1 = vi.fn(() => {
throw new Error('Listener error');
});
const listener2 = vi.fn();
emitter.on('test', listener1);
emitter.on('test', listener2);
expect(() => emitter.emit('test')).toThrow('Listener error');
- // Note: listener2 won't be called because listener1 throws
+ // listener2 won't be called because listener1 throws first
+ expect(listener2).not.toHaveBeenCalled();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should not fail if a listener throws an error', () => { | |
| const emitter = new EventEmitter(); | |
| const listener1 = vi.fn(() => { | |
| throw new Error('Listener error'); | |
| }); | |
| const listener2 = vi.fn(); | |
| emitter.on('test', listener1); | |
| emitter.on('test', listener2); | |
| expect(() => emitter.emit('test')).toThrow('Listener error'); | |
| // Note: listener2 won't be called because listener1 throws | |
| }); | |
| it('should propagate errors thrown by listeners', () => { | |
| const emitter = new EventEmitter(); | |
| const listener1 = vi.fn(() => { | |
| throw new Error('Listener error'); | |
| }); | |
| const listener2 = vi.fn(); | |
| emitter.on('test', listener1); | |
| emitter.on('test', listener2); | |
| expect(() => emitter.emit('test')).toThrow('Listener error'); | |
| // listener2 won't be called because listener1 throws first | |
| expect(listener2).not.toHaveBeenCalled(); | |
| }); |
🤖 Prompt for AI Agents
In
@greatfrontend/gfe-75/javascript-utilities/event-emitter/event-emitter.test.ts
around lines 239 - 251, The test name is misleading and missing an assertion for
listener2: rename the test to reflect that errors from listeners propagate
(e.g., "should propagate errors from a listener and prevent subsequent
listeners") and keep the existing expect(() =>
emitter.emit('test')).toThrow('Listener error'); then add an assertion verifying
listener2 was not called (expect(listener2).not.toHaveBeenCalled()); reference
EventEmitter, emit, on, listener1 and listener2 to locate where to change.

Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.