Skip to content

Conversation

@bkonyi
Copy link
Contributor

@bkonyi bkonyi commented Sep 30, 2025

This change introduces PersistentPreferences, which allows for the widget previewer to save settings to disk.

PersistentPreferences makes use of the existing ~/.flutter-devtools directory used by DevTools for the same purpose, writing preferences in JSON format to ~/.flutter-devtools/.widget-preview.

This change introduces `PersistentPreferences`, which allows for the
widget previewer to save settings to disk.

`PersistentPreferences` makes use of the existing `~/.flutter-devtools`
directory used by DevTools for the same purpose, writing preferences in
JSON format to `~/.flutter-devtools/.widget-preview`.
@bkonyi bkonyi requested a review from DanTup September 30, 2025 15:25
@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Sep 30, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces persistence for widget preview settings by creating a PersistentPreferences class. The implementation is sound, but there are a few areas for improvement regarding error handling, API design, and test suite hygiene. Specifically, I've pointed out a case of silent error handling, a bug in the remove method, an unintuitive API return value, and some duplicated test code. Addressing these points will improve the robustness and maintainability of the new feature.


try {
const jsonEncoder = JsonEncoder.withIndent(' ');
file.writeAsStringSync('${jsonEncoder.convert(_map)}\n');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We spawn a widget preview for each "window" in VS Code. If you have multiple windows open (which I don't think is uncommon), they could overwrite each others files here. It's a bit of an edge case, but it might be confusing. I wonder if we should re-read them on-change and then persist with just the one value updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the only correct way to handle this is to register a file watcher that's responsive to changes in the preferences file, which then sends DTD events to the widget preview scaffold so it can update its state immediately.

I pulled most of this implementation from DevTools, so it'll have the same issue. Maybe this is something we revisit?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree watching the file is probably overkill, but what about re-reading the file just before modifying, so if another process had saved a value, it's preserved rather than wipe? (just an idea, not suggesting it should be done to land this)

@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 1, 2025
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 1, 2025
@auto-submit
Copy link
Contributor

auto-submit bot commented Oct 1, 2025

autosubmit label was removed for flutter/flutter/176289, because - The status or check suite Linux_android_emu native_assets_android has failed. Please fix the issues identified (or deflake) before re-applying this label.

@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 1, 2025
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 1, 2025
@auto-submit
Copy link
Contributor

auto-submit bot commented Oct 1, 2025

autosubmit label was removed for flutter/flutter/176289, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 1, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Oct 1, 2025
Merged via the queue into master with commit 23a9ce3 Oct 1, 2025
155 checks passed
@auto-submit auto-submit bot deleted the persistent_properties_previewer branch October 1, 2025 23:52
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 1, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Oct 2, 2025
flutter/flutter@7811e89...65aca36

2025-10-02 [email protected] Roll Skia from 257c1f94afaa to 05c1f5803415 (4 revisions) (flutter/flutter#176402)
2025-10-02 [email protected] [ Widget Preview ] Fix resolution for workspace "hosted" dependencies (flutter/flutter#176358)
2025-10-02 [email protected] Roll Skia from b5d8ae8d3410 to 257c1f94afaa (6 revisions) (flutter/flutter#176389)
2025-10-02 [email protected] Delete Skia-specific performance overlay implementation (flutter/flutter#176364)
2025-10-02 [email protected] Roll Fuchsia Linux SDK from 1Ai6VL4vb_GdGnWhg... to Vnoygds8HtDUvGLCK... (flutter/flutter#176381)
2025-10-01 [email protected] [ Widget Preview ] Persist "Filter by Selected File" toggle (flutter/flutter#176289)
2025-10-01 [email protected] Roll Skia from c44a36470d07 to b5d8ae8d3410 (5 revisions) (flutter/flutter#176367)
2025-10-01 [email protected] Reapply "Update the AccessibilityPlugin::Announce method to account f… (flutter/flutter#176107)
2025-10-01 [email protected] Roll Dart SDK from 8ffec1435cf3 to 4f90a06328cb (3 revisions) (flutter/flutter#176369)
2025-10-01 [email protected] [ Tool / l10n ] Fix issue where localization generator assumed current directory was the target project (flutter/flutter#175881)
2025-10-01 [email protected] Make sure that a DateRangePickerDialog doesn't crash in 0x0 environments (flutter/flutter#173754)
2025-10-01 [email protected] Make sure that a DrawerButton doesn't crash in 0x0 environment (flutter/flutter#172948)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
mboetger pushed a commit to mboetger/flutter that referenced this pull request Oct 7, 2025
…176289)

This change introduces `PersistentPreferences`, which allows for the
widget previewer to save settings to disk.

`PersistentPreferences` makes use of the existing `~/.flutter-devtools`
directory used by DevTools for the same purpose, writing preferences in
JSON format to `~/.flutter-devtools/.widget-preview`.
okorohelijah pushed a commit to okorohelijah/flutter that referenced this pull request Oct 7, 2025
…176289)

This change introduces `PersistentPreferences`, which allows for the
widget previewer to save settings to disk.

`PersistentPreferences` makes use of the existing `~/.flutter-devtools`
directory used by DevTools for the same purpose, writing preferences in
JSON format to `~/.flutter-devtools/.widget-preview`.
reidbaker pushed a commit to AbdeMohlbi/flutter that referenced this pull request Dec 10, 2025
…176289)

This change introduces `PersistentPreferences`, which allows for the
widget previewer to save settings to disk.

`PersistentPreferences` makes use of the existing `~/.flutter-devtools`
directory used by DevTools for the same purpose, writing preferences in
JSON format to `~/.flutter-devtools/.widget-preview`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants