Skip to content

[rfw] Opt out of icon tree shaking#11216

Open
dcharkes wants to merge 1 commit intomainfrom
rfw-tree-shaking-icons-opt-out
Open

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

Conversation

@dcharkes
Copy link

@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
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

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