Add eager/lazy/disabled options for Widget Preview #5840
Conversation
|
@codex review /gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds a new setting dart.flutterWidgetPreview to control the startup behavior of the Widget Preview feature, with options for eager, lazy, or disabled. The default is changed to lazy to reduce resource consumption, which is a sensible improvement. The implementation for lazy-loading the preview server seems robust, correctly handling different scenarios like sidebar and "beside" views. I've left one comment regarding handling changes to the new setting.
There was a problem hiding this comment.
Pull request overview
This PR adds a new configuration setting dart.flutterWidgetPreview to control the Widget Preview server startup behavior, with three options: startEagerly, startLazily, and disabled. The default is changed from eager to lazy startup to reduce memory consumption and address performance issues. The lazy initialization defers server startup until the preview is actually needed (when the sidebar is resolved or when explicitly shown).
Key Changes:
- Server initialization moved from constructor to a lazy
startServer()method - Added
PromiseCompleterto handle async server access - New configuration option with three modes for startup behavior
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/extension/flutter/widget_preview/widget_preview_manager.ts | Refactored server initialization to support lazy/eager/disabled modes; moved server creation from constructor to startServer() method |
| src/extension/flutter/widget_preview/webviews.ts | Added onDidResolve event to WidgetPreviewSidebarView to trigger lazy server startup when sidebar is resolved |
| src/extension/extension.ts | Added check for disabled mode and passed new behavior parameter to FlutterWidgetPreviewManager |
| src/extension/config.ts | Added new flutterWidgetPreview configuration property with default value startLazily |
| package.json | Added configuration schema for dart.flutterWidgetPreview with three enum values and descriptions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Codex Review: Didn't find any major issues. Swish! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This adds a setting to control whether the widget preview starts eagerly, lazily, or not at all.
For now, I'm changing the default to lazy, since it can consume a lot of memory and there have been a few issues - but we can consider switching it to eagerly (to make it load faster the first time if you do use it) if that becomes a better trade-off than it is today (FYI @bkonyi).
Fixes #5814