Skip to content

Conversation

@bkonyi
Copy link
Contributor

@bkonyi bkonyi commented Aug 21, 2025

Instead of crashing, invalid previews should just be ignored.

Fixes #173710

…ams to a `Preview`

Instead of crashing, invalid previews should just be ignored.

Fixes #173710
@bkonyi bkonyi requested a review from matanlurey August 21, 2025 20:57
@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Aug 21, 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 fixes a crash that occurs when a Preview annotation is used with non-constant parameters. The fix involves checking if computeConstantValue() returns null and, if so, ignoring the invalid preview instead of crashing. The changes are applied to both single Preview annotations and MultiPreview annotations. Additionally, new tests are added to verify this behavior, and existing test code is refactored for better maintainability by using a mixin. My feedback includes a suggestion to improve code clarity and robustness in one of the changed utility functions.

@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 21, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Aug 21, 2025
Merged via the queue into master with commit c326541 Aug 22, 2025
151 checks passed
@auto-submit auto-submit bot deleted the fix_issue_173710 branch August 22, 2025 00:27
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 22, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 22, 2025
SydneyBao pushed a commit to SydneyBao/flutter that referenced this pull request Aug 22, 2025
…ams to a `Preview` (flutter#174242)

Instead of crashing, invalid previews should just be ignored.

Fixes flutter#173710
SydneyBao pushed a commit to SydneyBao/flutter that referenced this pull request Aug 22, 2025
…ams to a `Preview` (flutter#174242)

Instead of crashing, invalid previews should just be ignored.

Fixes flutter#173710
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 22, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Aug 22, 2025
flutter/flutter@d2ac021...26bb33b

2025-08-22 [email protected] [HCPP] Clean up overlay layer when last frame had overlay content and current doesn't (flutter/flutter#173881)
2025-08-22 [email protected] Migrate more files to use WidgetStateProperty (flutter/flutter#174176)
2025-08-22 [email protected] Roll Skia from 75fef9fb3ed7 to cb15e1452399 (1 revision) (flutter/flutter#174255)
2025-08-22 [email protected] Roll Skia from 006241a7fbe1 to 75fef9fb3ed7 (1 revision) (flutter/flutter#174254)
2025-08-22 [email protected] Skip wasm build when dry run is disabled and --wasm is not specified. (flutter/flutter#174184)
2025-08-22 [email protected] Roll Dart SDK from c153c5259e62 to 4f9623f024ab (2 revisions) (flutter/flutter#174250)
2025-08-22 [email protected] Roll Skia from d70087007490 to 006241a7fbe1 (2 revisions) (flutter/flutter#174252)
2025-08-22 [email protected] Roll Skia from c09589f7ca69 to d70087007490 (22 revisions) (flutter/flutter#174245)
2025-08-21 [email protected] [ Widget Preview ] Add regression test for issue 173895 (flutter/flutter#174037)
2025-08-21 [email protected] Improve xcresult comment and naming (flutter/flutter#173129)
2025-08-21 [email protected] Update `.gemini/styleguide.md` to encourage `master`-only (flutter/flutter#174065)
2025-08-21 [email protected] [ Widget Preview ] Fix crash when attempting to provide non-const params to a `Preview` (flutter/flutter#174242)
2025-08-21 [email protected] Roll Skia from 721e68fe652a to c09589f7ca69 (12 revisions) (flutter/flutter#174162)
2025-08-21 [email protected] Roll Dart SDK from 0d0a0c394381 to c153c5259e62 (7 revisions) (flutter/flutter#174235)
2025-08-21 [email protected] [ Tool ] Throw `ToolExit` when asset entries use absolute paths (flutter/flutter#174230)
2025-08-21 [email protected] Roll Dart SDK from 0d0a0c394381 to c153c5259e62 (7 revisions) (flutter/flutter#174227)
2025-08-21 [email protected] [ Tool ] Cleanup widget preview and frontend server shutdown (flutter/flutter#173863)
2025-08-21 [email protected] Use an alternative to `git describe` for `master` version resolution (flutter/flutter#174088)
2025-08-21 [email protected] Report a correct display ID in the window metrics event on win32 (flutter/flutter#174156)
2025-08-21 [email protected] Revert "Update the AccessibilityPlugin::Announce method to account fo… (flutter/flutter#174223)

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],[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 Sep 18, 2025
…ams to a `Preview` (flutter#174242)

Instead of crashing, invalid previews should just be ignored.

Fixes flutter#173710
korca0220 pushed a commit to korca0220/flutter that referenced this pull request Sep 22, 2025
…ams to a `Preview` (flutter#174242)

Instead of crashing, invalid previews should just be ignored.

Fixes flutter#173710
Jaineel-Mamtora pushed a commit to Jaineel-Mamtora/flutter_forked that referenced this pull request Sep 24, 2025
…ams to a `Preview` (flutter#174242)

Instead of crashing, invalid previews should just be ignored.

Fixes flutter#173710
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 12, 2025
lucaantonelli pushed a commit to lucaantonelli/flutter that referenced this pull request Nov 21, 2025
…ams to a `Preview` (flutter#174242)

Instead of crashing, invalid previews should just be ignored.

Fixes flutter#173710
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.

Error while modifying Widget Preview annotations

2 participants