fix(react): memoize event listener

[jog1t]

Fixes KIT-142

TL;DR

Fixed event handler reference issues in React framework and improved type definitions for actor state.

What changed?

• Created a new ActorsStateDerived type to improve type safety and reduce code duplication
• Removed unnecessary biome-ignore comments
• Fixed a critical issue in the React useEvent hook where handler references weren't being properly maintained between renders
  • Added a useRef to store the latest handler function
  • Created a stable event handler function that calls the referenced handler
  • Updated dependency array to include actorState.hash for proper cleanup

How to test?

1. Create a component that uses the useEvent hook with a handler that depends on props or state
2. Verify that the event handler correctly uses the latest props/state values when triggered
3. Check that event listeners are properly cleaned up when dependencies change

Why make this change?

The previous implementation of useEvent had a closure issue where the handler function would capture variables from the render it was defined in, not updating when those variables changed in subsequent renders. This could lead to stale data being used in event handlers.

Additionally, the type improvements make the codebase more maintainable by centralizing the actor state derived type definition and removing redundant code.

[claude]

Claude finished @jog1t's task —— View job

---

PR Review: fix(react): memoize event listener

Todo List

• Read and analyze changes in framework-base/lib/mod.ts
• Read and analyze changes in react/src/mod.tsx
• View git diff to understand exact changes made
• Evaluate code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Review security concerns
• Consider test coverage needs
• Provide comprehensive feedback

Summary

This PR improves performance by memoizing event listener handlers in the React integration and adds a type alias to reduce code duplication in the framework base.

Review Feedback

✅ Code Quality & Best Practices

Framework Base (packages/frameworks/framework-base/lib/mod.ts):

• Good: Added ActorsStateDerived<Registry, ActorName> type alias eliminates code duplication and improves maintainability
• Good: Cleaned up unnecessary biome-ignore comments
• Good: Type definitions are well-structured and properly documented

React Integration (packages/frameworks/react/src/mod.tsx):

• Good: Proper use of useRef for memoization follows React best practices
• Good: Changed from arrow function to function declaration for useEvent (consistent with other function declarations)
• Good: Added proper handler synchronization with useEffect

✅ Performance Considerations

Excellent performance improvements:

• Main Fix: Memoizes event handlers using useRef to prevent unnecessary event listener re-subscriptions when handler function reference changes
• Before: Every render with a new handler function (common with inline functions/callbacks) would cause the event listener to be removed and re-added
• After: Handler is memoized and only the internal reference is updated, preventing costly re-subscriptions
• Impact: Significant performance improvement for components that frequently re-render with event handlers

Dependency array optimization:

• Good: Removed handler from dependencies array (line 80) since it's now memoized
• Good: Added actorState.hash to dependencies to ensure event listeners are updated when the actor instance changes

⚠️ Potential Issues

Minor concerns:

1. Dependency Array (react/src/mod.tsx:80): The dependency array includes actorState.isConnected and actorState.hash. Consider if actorState.isConnected is necessary since the effect already guards with if (!actorState?.connection) return.

2. Type Safety: The eventHandler wrapper function uses any[] parameters. While this matches the existing pattern, consider if stronger typing could be achieved.

✅ Security Concerns

No security issues identified:

• Changes don't introduce any new attack vectors
• Event handler memoization is a safe optimization
• Type definitions maintain existing constraints

⚠️ Test Coverage Recommendations

Missing test scenarios:

1. Event Handler Memoization: Test that event listeners aren't recreated when component re-renders with same handler logic but different function reference
2. Handler Updates: Verify that when the actual handler logic changes, the event listener properly uses the new handler
3. Actor Instance Changes: Test that event listeners are properly updated when actorState.hash changes

Suggested test addition:

// Test that handler isn't recreated unnecessarily
it('should memoize event handlers to prevent unnecessary re-subscriptions', () => {
  const { rerender } = render(<TestComponent />);
  const firstSubscription = mockConnection.on.mock.calls.length;
  
  rerender(<TestComponent />); // Re-render with new handler reference
  
  // Should not create new subscription
  expect(mockConnection.on.mock.calls.length).toBe(firstSubscription);
});

📋 Summary

Strengths:

• ✅ Solid performance optimization addressing a real React performance issue
• ✅ Clean type refactoring reduces code duplication
• ✅ Follows React best practices for memoization
• ✅ No breaking changes to public API

Recommendations:

• Consider reviewing if actorState.isConnected dependency is needed
• Add tests for the memoization behavior
• Consider stronger typing for event handler parameters in future iterations

Overall Assessment: Approved ✅
This is a well-implemented performance fix that addresses unnecessary event listener churn in React components. The changes are minimal, focused, and follow established patterns.

[jog1t]

• feat(core): subscribe to events before connect #1146
• fix(react): memoize event listener #1142 👈 (View in Graphite)
• main

---

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

• merge queue - adds this PR to the back of the merge queue
• hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[linear]

KIT-142 Fix useEvent resubscribing every time component rerenders

[pkg-pr-new]

Open in StackBlitz

@rivetkit/actor

pnpm add https://pkg.pr.new/rivet-gg/rivetkit/@rivetkit/actor@1142

@rivetkit/core

pnpm add https://pkg.pr.new/rivet-gg/rivetkit/@rivetkit/core@1142

@rivetkit/db

pnpm add https://pkg.pr.new/rivet-gg/rivetkit/@rivetkit/db@1142

rivetkit

pnpm add https://pkg.pr.new/rivet-gg/rivetkit@1142

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-gg/rivetkit/@rivetkit/cloudflare-workers@1142

@rivetkit/redis

pnpm add https://pkg.pr.new/rivet-gg/rivetkit/@rivetkit/redis@1142

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-gg/rivetkit/@rivetkit/framework-base@1142

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-gg/rivetkit/@rivetkit/react@1142

commit: f8d641c