Skip to content

Conversation

@doppioandante
Copy link
Contributor

@doppioandante doppioandante commented Oct 26, 2025

I've been using Dance quite a lot recently, but sometimes it started to lag excessively, particularly after I would trigger a conflict between code and dance handling the cursor.
I'm not keen on delving into a lot of typescript and I'm not familiar with the vscode extension model, so I just asked Claude to look around for possible causes. The === -> = looks like a legit mistype, but I have no clue about the rest.
My limited testing for now makes me think that it solved the issue; can somebody with experience with the codebase chime in and review it? The first commit should directly address the issue, the second one is more things there were found.

Probably fixes #343 (comment) (or #343 altogether)

doppioandante and others added 3 commits October 26, 2025 20:40
Fixed three critical bugs that caused progressive performance degradation:
- Extension.dispose() now properly cleans up subscriptions and resources to prevent event listener accumulation on reload
- Fixed memory leak in editors.ts where array length comparison was used instead of assignment (length === 0 -> length = 0)
- Fixed selection update race condition by deferring reveal() to prevent cursor jumping during navigation

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Fixed several secondary performance and reliability issues:
- Replaced setTimeout with queueMicrotask in mode changes for predictable event listener setup
- Fixed stale document references by capturing document at function start in selectionsToCharacterMode and selectionsFromCharacterMode
- Added disposal guard to prevent initialization race in Editors class

These changes improve stability and prevent potential edge cases that could contribute to performance degradation.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@71
Copy link
Owner

71 commented Nov 1, 2025

Hey there! Thanks for the investigation and fix.

Some of the findings don't seem to make a lot of sense to me (like replacing a lazy document load with an eager one). That said, the finding with === 0 looks really good1, I'll test it locally and see if it fixes things. The extension not properly disposing is also a good find, although less critical (only happens on extension being disabled). I found another potential memory leak that I'll investigate at the same time.

Edit: I'm actually not sure the memory leak comes from here. We do leak memory there, but the array is cleared every time we open an editor, so in practice the array stays very small.

Footnotes

  1. I'm surprised that neither TypeScript nor eslint flagged this, it's pretty obvious that it's not intentional to me.

@71 71 mentioned this pull request Nov 8, 2025
@71
Copy link
Owner

71 commented Nov 15, 2025

I looked at the changes and ended up submitting #416 with some of them. In my case, this didn't solve the performance problem, but it's still nice to get rid of some memory leaks. Thanks!

@71 71 closed this Nov 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Keypresses race

2 participants