fix: add idle watcher interval cleanup and resource management#5770
fix: add idle watcher interval cleanup and resource management#5770DhyaniKavya wants to merge 4 commits intosugarlabs:masterfrom
Conversation
- Store interval ID in _idleWatcherIntervalId property - Store reset handler in _resetIdleTimer for cleanup - Prevent duplicate intervals with clearInterval check - Add cleanupIdleWatcher() method to remove interval and event listeners - No behavior changes to idle detection or FPS throttling
|
✅ All Jest tests passed! This PR is ready to merge. |
|
@walterbender @vanshika2720 ES lint checks are not passing. I ran "npx prettier --write js/activity.js" but still checks are not passing . Can you suggest something? |
|
✅ All Jest tests passed! This PR is ready to merge. |
|
✅ All Jest tests passed! This PR is ready to merge. |
|
@walterbender @vanshika2720 sorry to ping you frequently . But now all the checks have been passed , there was some issue with the CI, i solved that now. Please review the changes if they are preferable. i am open to address and take notes of what you say. Thankyou |
|
@DhyaniKavya I’ve reviewed the changes — the cleanup and interval handling look good overall. |
|
@vanshika2720 Thanks for reviewing and for the helpful suggestion Appreciate the feedback! |
|
✅ All Jest tests passed! This PR is ready to merge. |
|
@omsuneri please review the changes if anything required , i am open to address them. Thankyou |
|
@vanshika2720 please check if the changes are good for approval or need more changes |
|
@DhyaniKavya I’ve reviewed the changes — they look good and properly handle the cleanup. Safe for approval from my side. |
|
Thankyou @vanshika2720 for approving. |
|
@omsuneri This PR is ready for review, need approval from your side . Thankyou |
2 participants
issue : #5768
Problem:-
The idle watcher in js/activity.js creates a setInterval that runs every second to monitor inactivity and adjust the frame rate. The interval ID is not stored, and the activity reset event listeners are attached using a handler reference that cannot be removed.
As a result:
-the interval cannot be cleared if the activity is reinitialized
-duplicate intervals could accumulate in future lifecycle scenarios
-event listeners cannot be removed during teardown
-unnecessary resource usage may occur in long sessions or development reloads
-While this does not cause immediate user-visible errors, it represents incomplete lifecycle and resource management.
Solution:-
-This PR enables proper lifecycle cleanup while preserving all existing behavior:
-stores the idle watcher interval ID
-prevents duplicate intervals from being created
-stores the reset handler for listener removal
-adds a cleanup method to remove the interval and listeners
-does not change idle detection logic or behavior
Changes Made:-
-File modified:
->js/activity.js
->store interval ID for cleanup
->prevent duplicate interval creation
->store reset handler reference
->add cleanupIdleWatcher() method for lifecycle cleanup
Testing Performed:-
-Manual Verification
-Idle throttling activates after inactivity
-Frame rate restores on activity
-Playback and interaction behavior unchanged
-No console errors observed
-Automated Testing
Jest: ✅ all tests passed⚠️ pre-existing repository configuration warning (no new lint errors introduced)
Prettier: ✅ formatting verified
ESLint:
Impact:-
-Improves resource and lifecycle management
-Prevents duplicate idle watcher intervals
-Enables safe cleanup for future modularization and testing
-Improves maintainability and long-session efficiency
-No breaking changes
Notes:-
This change is intentionally minimal and non-breaking. The cleanup method is opt-in and does not alter current behavior. It provides proper lifecycle handling for future use cases such as testing, development reloads, or modular integration.
Happy to adjust if maintainers prefer a different approach.