-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: prevent UI freeze when showing save dialogs (#6870) #6871
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
Conversation
- Created safeDialogs utility with non-blocking wrappers for showSaveDialog and showOpenDialog - Updated all save/open dialog calls to use the safe wrappers - Uses setImmediate to defer dialog calls to next event loop iteration - Prevents VSCode UI from freezing on macOS with certain configurations Fixes #6870
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.
Apparently I can review my own code but can't prevent it from freezing the UI in the first place.
| * @returns Promise that resolves to the selected URI or undefined if cancelled | ||
| */ | ||
| export async function showSaveDialogSafe(options: vscode.SaveDialogOptions): Promise<vscode.Uri | undefined> { | ||
| // Use setImmediate to defer the dialog call to the next iteration of the event loop |
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.
Would it be helpful to add a comment explaining why setImmediate specifically prevents the UI freeze? This would help future maintainers understand the mechanism behind this fix.
For example: 'setImmediate schedules the callback to execute after the current event loop completes, allowing the UI thread to process pending events and remain responsive.'
| resolve(result) | ||
| } catch (error) { | ||
| console.error("Error showing save dialog:", error) | ||
| resolve(undefined) |
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.
Is resolving to undefined on error intentional? The error is logged, but callers can't distinguish between user cancellation and an actual error. Would it be better to re-throw the error after logging, or is this defensive behavior by design?
| @@ -0,0 +1,46 @@ | |||
| import * as vscode from "vscode" | |||
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 new utility module should have unit tests to verify the non-blocking behavior and error handling. Could we add tests to ensure:
- The functions are called with the correct options
- Results are properly returned
- Errors are handled gracefully
- The deferred execution via setImmediate works as expected
|
|
||
| import * as vscode from "vscode" | ||
| import { z, ZodError } from "zod" | ||
| import { showSaveDialogSafe, showOpenDialogSafe } from "../../utils/safeDialogs" |
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 test file src/core/config/tests/importExport.spec.ts still mocks vscode.window.showOpenDialog and vscode.window.showSaveDialog directly. Should the tests be updated to mock the new safe wrapper functions instead?
|
I also noticed that |
|
Not an issue, closing |
Description
This PR fixes an issue where the VSCode UI would freeze when the save dialog appears after edits in Code mode, particularly on macOS with certain configurations.
Problem
Solution
Created a utility module
safeDialogs.tsthat wraps VSCode's dialog functions in a non-blocking way:setImmediateto defer dialog calls to the next iteration of the event loopChanges
src/utils/safeDialogs.tswith safe wrappers forshowSaveDialogandshowOpenDialogsrc/integrations/misc/image-handler.tssrc/integrations/misc/export-markdown.tssrc/core/webview/webviewMessageHandler.tssrc/core/config/importExport.tsTesting
Fixes #6870
Important
Fixes UI freeze issue in VSCode by introducing non-blocking dialog wrappers in
safeDialogs.tsand updating relevant files to use them.safeDialogs.tsto wrapvscode.window.showSaveDialogandvscode.window.showOpenDialogin a non-blocking manner usingsetImmediate.importExport.ts,webviewMessageHandler.ts,export-markdown.ts,image-handler.tsupdated to useshowSaveDialogSafeandshowOpenDialogSafe.This description was created by
for c101ed9. You can customize this summary. It will automatically update as commits are pushed.