fluttercommunity / font_awesome_flutter

The Font Awesome Icon pack available as Flutter Icons
Other
831 stars 233 forks source link

Flutter Build Option `--tree-shake-icons` fails due to font_awesome_flutter-10.2.1 #228

Closed churchianity closed 1 year ago

churchianity commented 1 year ago

If you build a flutter application with the option --tree-shake-icons for the web, you get some errors coming from icon_data.dart. Tree shaking only works for icon data that is const, and a few IconData subclasses introduced by font_awesome_flutter are not considered const.

Steps to reproduce the behavior:

Expected behavior To be fair, it's not necessarily a 'bug', but being able to tree shake icons is very valuable for minimizing bundle size, and many people will likely be serving more icons that they need to while this behavior exists.

Screenshots image

Desktop (please complete the following information):

michaelspiss commented 1 year ago

Thanks for reporting this. It would be a bug, as we officially support icon tree shaking. However, I cannot reproduce this. The specified lines also all contain a const call, so I cannot see how there would be an error.

Building the example app runs without errors (tree shaking is enabled by default):

C:\Users\Michael\IdeaProjects\font_awesome_flutter\example>flutter build apk

 Building with sound null safety

Calling mockable JAR artifact transform to create file: C:\Users\Michael\.gradle\caches\transforms-2\files-2.1\43bf54ced26cecd29a682cf452e6d9a2\android.jar with input C:\Users\Michael\AppData\Local\Android\sdk\platforms\android-28\android.jar
Removed unused resources: Binary resource data reduced from 47KB to 38KB: Removed 19%
Running Gradle task 'assembleRelease'...                          238.8s
√  Built build\app\outputs\flutter-apk\app-release.apk (18.3MB).

This is an odd one, especially since it happens on two separate machines. Could you please attach the output of flutter doctor -v and maybe run flutter clean and flutter pub get again, to make sure this is not some cached old version?

churchianity commented 1 year ago

image

image

No dice, it seems. I can try and make a minimally reproducible example, but that might be a challenge.

michaelspiss commented 1 year ago

Can you try building the example app? To do this, clone this repository and go to the example subfolder. Then run flutter pub get and flutter build apk

churchianity commented 1 year ago

Is there a reason why I'd need to build the apk specifically? I'm only really targeting the web at the moment, and I don't have a java installation to build with the apk right now. If it's necessary I could get it.

Running flutter build web in the example directory works. However, if I run flutter build web --tree-shake-icons in the example directory, it does not work, and I get a message consistent with what I had before:

image

michaelspiss commented 1 year ago

Icon tree shaking is disabled for web builds - so adding "--tree-shake-icons" breaks your build. This is an open issue of the flutter framework.

But this may already fix your problem then

churchianity commented 1 year ago

My apologies. Thanks for the link. I feel a bit misled that the option doesn't spit out 'not supported', but that is certainly not your problem.

Thanks for the help.

michaelspiss commented 1 year ago

No worries, I wasn't aware of this limitation either. I would have expected an error message as well. Good luck with your project!

churchianity commented 1 year ago

With flutter 3.3.10, they added the ability to tree shake fonts from web builds, with the normal flag of --tree-shake-icons.

You can see it in the notes here: https://github.com/flutter/packages/commit/13fb549af668ff8957eef526573756eff4ef42ab

And the specific issue: https://github.com/flutter/flutter/pull/115886

image

I got the same stuff I got before it was even supported... a bit mysterious.

michaelspiss commented 1 year ago

Flutter 3.3.10 is not released yet, the latest stable version is 3.3.9. If you want to use upcoming features, switch to the beta branch first.

I'd personally just wait for flutter 3.3.10 to release, so they can figure out the quirks first and you don't end up with an unstable system though.

churchianity commented 1 year ago

image

I may be confused, but I'm on the stable channel, and have version 3.3.10

In the flutter discord releases channel, they posted this yesterday: image

michaelspiss commented 1 year ago

Ah so they just haven't updated the web docs then - that's where I looked as I currently don't have access to a PC. Are you sure web icon tree shaking is included in 3.3.10? It would be kinda strange to have a feature added in a Bugfix release and I cannot find anything about it in the release notes. I'd assume web icon tree shaking to be available with 3.4.0 if it has already been merged.

churchianity commented 1 year ago

Yeah, my mistake after all (again).

image

It appears to be planned for 3.7.

michaelspiss commented 1 year ago

Thanks for the research though, those are great news for the future!

5hirish commented 1 year ago

Any idea when 3.7 will be released?

michaelspiss commented 1 year ago

No, sorry