Skip to content

[rfw] Opt out of icon tree shaking#11216

Merged
dcharkes merged 1 commit intomainfrom
rfw-tree-shaking-icons-opt-out
Mar 11, 2026
Merged

[rfw] Opt out of icon tree shaking#11216
dcharkes merged 1 commit intomainfrom
rfw-tree-shaking-icons-opt-out

Conversation

@dcharkes
Copy link
Contributor

@dcharkes dcharkes commented Mar 10, 2026

We are introducing the @mustBeConst annotation on IconData parameters that must be const in order for the icon tree shaker to work:

RFW does not support tree-shaking.

This PR adds the lint ignores with an explicit comment that the lint ignore is intentional.

@dcharkes dcharkes requested a review from Hixie as a code owner March 10, 2026 08:26
@github-actions github-actions bot added the p: rfw Remote Flutter Widgets label Mar 10, 2026
Copy link

@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 opts out of icon tree shaking for RFW by adding ignore comments for the non_const_argument_for_const_parameter lint. This is necessary because RFW creates IconData instances with dynamic data, which is incompatible with the new @mustBeConst annotations on IconData parameters. The change is correct in principle, but it seems to be missing an ignore for the matchTextDirection parameter, which is also marked as @mustBeConst.

icon,
// ignore: non_const_argument_for_const_parameter
fontFamily: source.v<String>([...key, 'fontFamily']),
matchTextDirection: source.v<bool>([...key, 'matchTextDirection']) ?? false,

Choose a reason for hiding this comment

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

high

It seems you've missed adding an ignore for the matchTextDirection parameter. According to the Flutter PR you linked, matchTextDirection is also annotated with @mustBeConst. Since the value passed here is not a compile-time constant, it will also trigger the non_const_argument_for_const_parameter lint and should be ignored.

Suggested change
matchTextDirection: source.v<bool>([...key, 'matchTextDirection']) ?? false,
// ignore: non_const_argument_for_const_parameter
matchTextDirection: source.v<bool>([...key, 'matchTextDirection']) ?? false,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mr AI bot, you are talking BS:

image

Copy link
Contributor

@Hixie Hixie left a comment

Choose a reason for hiding this comment

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

LGTM

@dcharkes dcharkes merged commit a6542ce into main Mar 11, 2026
81 checks passed
@dcharkes dcharkes deleted the rfw-tree-shaking-icons-opt-out branch March 11, 2026 07:05
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 11, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 11, 2026
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Mar 11, 2026
flutter/packages@ee460d6...ecace66

2026-03-11 srawlins@google.com [pigeon] Produce a helpful error for bad
method return type (flutter/packages#11204)
2026-03-11 srawlins@google.com [pigeon] Use hasLength and isEmpty in
tests for better failure messages (flutter/packages#11205)
2026-03-11 dacoharkes@google.com [rfw] Opt out of icon tree shaking
(flutter/packages#11216)
2026-03-11 srawlins@google.com [pigeon] Tidy imports and "ignore"
comments (flutter/packages#11149)
2026-03-11 49699333+dependabot[bot]@users.noreply.github.com
[dependabot]: Bump org.jetbrains.kotlin:kotlin-bom from 2.2.21 to 2.3.10
in /packages/pigeon/platform_tests/test_plugin/android
(flutter/packages#10984)
2026-03-11 srawlins@google.com [pigeon] Improve casting and
nullability-handling in generated code (flutter/packages#11163)
2026-03-10 engine-flutter-autoroll@skia.org Roll Flutter from
2ec61af to 195ae7b (36 revisions) (flutter/packages#11222)
2026-03-10 git@reb0.org [vector_graphics] Respect BoxFit parameter with
viewbox (flutter/packages#11012)
2026-03-10 stuartmorgan@google.com Add AI contribution guidelines to PR
checklist (flutter/packages#11195)
2026-03-10 zezohassam@gmail.com [video_player] Optimize caption
retrieval with binary search in VideoPlayerController
(flutter/packages#8347)

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-flutter-autoroll
Please CC flutter-ecosystem@google.com on the revert to ensure that a
human
is aware of the problem.

To file a bug in Flutter:
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
dcharkes added a commit to flutter/tests that referenced this pull request Mar 16, 2026
dcharkes added a commit to flutter/tests that referenced this pull request Mar 16, 2026
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Mar 24, 2026
2026-03-18 update, no push-back on the breaking changes. Migrations done
downstream:

* fluttercommunity/font_awesome_flutter#297
*
PiotrRogulski/kanji_app@76973c7
* flutter/packages#11216
* FaFre/WebLibre#214
* flutter/devtools#9718

-----------

Applying breaking changes:

* #181342
* #181344

See the issue descriptions for the motivation.

cc @Piinks 

It seems we have a non-const use in the Remote Flutter Widgets:


https://github.com/flutter/packages/blob/d0c7d1fad77331724882614345137221c77d5758/packages/rfw/lib/src/flutter/argument_decoders.dart#L801-L811

The package itself makes no mention of it not work with the Flutter Icon
Tree Shaker. If it's a know issue that the icon tree shaker is not
supported with `package:rfw` we can add `// ignore:
non_const_argument_for_const_parameter`.
(flutter/packages#10858)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

p: rfw Remote Flutter Widgets

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants