Prevent duplicate canvas event listener registration#618
Prevent duplicate canvas event listener registration#618tracygardner wants to merge 1 commit intomainfrom
Conversation
createEngine() is called on every code run (engine is disposed to null between runs), causing touchend, keydown, keyup, blur, and pointerup handlers to stack up indefinitely with no way to remove them. Guard registration with a one-time flag; the handlers are safe to reuse across runs since they access flock.scene lazily at call time. https://claude.ai/code/session_01QYFwXKPZrjm2RxVFKcKNvA
📝 WalkthroughWalkthroughThe ChangesEvent Listener Registration Guard
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@flock.js`:
- Around line 1283-1292: The touchend handler currently dereferences flock.scene
and flock.scene.activeCamera directly which can be null during dispose/re-init;
update the handler to guard against a missing scene before accessing
activeCamera and its inputs (e.g., check flock && flock.scene &&
flock.scene.activeCamera first), then continue with the existing logic that
reads inputs and calls flock._hardResetCameraControls; ensure the same
null-check pattern surrounds the input retrieval and subsequent use of
flock._hardResetCameraControls to avoid runtime exceptions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| const input = flock.scene.activeCamera.inputs?.attached?.pointers; | ||
| // Add null check for input itself | ||
| if ( | ||
| input && | ||
| (input._pointA !== null || | ||
| input._pointB !== null || | ||
| input._isMultiTouch === true) | ||
| ) { | ||
| flock._hardResetCameraControls(flock.scene.activeCamera, { | ||
| reattachDelayMs: 100, |
There was a problem hiding this comment.
Guard flock.scene before dereferencing in touchend handler.
flock.scene.activeCamera is accessed directly; during dispose/re-init windows flock.scene can be null, causing a runtime exception in the event handler.
Proposed fix
- const input = flock.scene.activeCamera.inputs?.attached?.pointers;
+ const activeCamera = flock.scene?.activeCamera;
+ const input = activeCamera?.inputs?.attached?.pointers;
// Add null check for input itself
if (
input &&
(input._pointA !== null ||
input._pointB !== null ||
input._isMultiTouch === true)
) {
- flock._hardResetCameraControls(flock.scene.activeCamera, {
+ flock._hardResetCameraControls(activeCamera, {
reattachDelayMs: 100,
noPreventDefault: true,
});
}📝 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.
| const input = flock.scene.activeCamera.inputs?.attached?.pointers; | |
| // Add null check for input itself | |
| if ( | |
| input && | |
| (input._pointA !== null || | |
| input._pointB !== null || | |
| input._isMultiTouch === true) | |
| ) { | |
| flock._hardResetCameraControls(flock.scene.activeCamera, { | |
| reattachDelayMs: 100, | |
| const activeCamera = flock.scene?.activeCamera; | |
| const input = activeCamera?.inputs?.attached?.pointers; | |
| // Add null check for input itself | |
| if ( | |
| input && | |
| (input._pointA !== null || | |
| input._pointB !== null || | |
| input._isMultiTouch === true) | |
| ) { | |
| flock._hardResetCameraControls(activeCamera, { | |
| reattachDelayMs: 100, |
🧰 Tools
🪛 GitHub Actions: Prettier / prettier
[warning] Prettier reported formatting issues.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@flock.js` around lines 1283 - 1292, The touchend handler currently
dereferences flock.scene and flock.scene.activeCamera directly which can be null
during dispose/re-init; update the handler to guard against a missing scene
before accessing activeCamera and its inputs (e.g., check flock && flock.scene
&& flock.scene.activeCamera first), then continue with the existing logic that
reads inputs and calls flock._hardResetCameraControls; ensure the same
null-check pattern surrounds the input retrieval and subsequent use of
flock._hardResetCameraControls to avoid runtime exceptions.
2 participants
Summary
Wrapped all canvas and window event listener registrations in a guard condition to prevent duplicate listener attachment when the initialization function is called multiple times.
Key Changes
flock._canvasListenersRegisteredflag to track whether listeners have already been registeredImplementation Details
The change uses a simple boolean flag (
flock._canvasListenersRegistered) to ensure that the event listener registration block only executes on the first initialization. This is a common pattern for preventing duplicate listener registration in scenarios where initialization functions may be called multiple times during the application lifecycle.https://claude.ai/code/session_01QYFwXKPZrjm2RxVFKcKNvA
Summary by CodeRabbit