The class IconData has a special constraints in its use due to the Flutter Icon Tree Shaker.
The class:
|
@immutable |
|
class IconData { |
|
/// Creates icon data. |
|
/// |
|
/// Rarely used directly. Instead, consider using one of the predefined icons |
|
/// like the [Icons] collection. |
|
/// |
|
/// The [fontFamily] argument is normally required when using custom icons. |
|
/// |
|
/// e.g. When using a [codePoint] from a `CustomIcons` font |
|
/// ```yaml |
|
/// fonts: |
|
/// - family: CustomIcons |
|
/// fonts: |
|
/// - asset: assets/fonts/CustomIcons.ttf |
|
/// ``` |
|
/// `IconData` usages should specify `fontFamily: 'CustomIcons'`. |
|
/// |
|
/// The [fontPackage] argument must be non-null when using a font family that |
|
/// is included in a package. This is used when selecting the font. |
|
/// |
|
/// Instantiating non-const instances of this class in your app will |
|
/// mean the app cannot be built in release mode with icon tree-shaking (it |
|
/// need to be explicitly opted out at build time). See [staticIconProvider] |
|
/// for more context. |
|
const IconData( |
|
this.codePoint, { |
|
this.fontFamily, |
|
this.fontPackage, |
|
this.matchTextDirection = false, |
|
this.fontFamilyFallback, |
|
}); |
The tree-shaker expects const instances with 3 specific fields with specific types:
|
Map<String, List<int>> _parseConstFinderResult(_ConstFinderResult constants) { |
|
final result = <String, List<int>>{}; |
|
for (final Map<String, Object?> iconDataMap in constants.constantInstances) { |
|
final Object? package = iconDataMap['fontPackage']; |
|
final Object? fontFamily = iconDataMap['fontFamily']; |
|
final Object? codePoint = iconDataMap['codePoint']; |
|
if ((package ?? '') is! String || (fontFamily ?? '') is! String || codePoint is! num) { |
|
throw IconTreeShakerException._( |
|
'Invalid ConstFinder result. Expected "fontPackage" to be a String, ' |
|
'"fontFamily" to be a String, and "codePoint" to be an int, ' |
|
'got: $iconDataMap.', |
|
); |
|
} |
However, the class is not marked final. Which makes it possible to subtype IconData in unexpected ways causing the tree-shaker to fail.
Giving packages the capability to tree-shake assets
We're working on a generalized mechanism of the icon tree shaker and underlying const finder that should give this kind of powers to packages. And in order for the generalized mechanism to be simpler to reason about we'd want to enforce users recording const instances to avoid having to deal with arbitrary subtypes of their classes marked for const-collection.
We can think of packages depending on each other as layering, and for each type marked for const-collection, it should be clear in which layer (package) the assets belonging to such type should be tree-shaken. In other words, which link hook will get the information about the recorded usages.
For the purposes of tree-shaking, layering should make it exactly clear which layer (package) is doing the tree shaking. Tree-shaking in either layer is fine:
- Letting the owner of
IconData, package:flutter (widgets.dart), do the tree shaking:
- If your
const IconDataBrand instances contain a const IconData instance, and the tree-shaking annotations are on IconData, then the package containing the definition of IconData is responsible for invoking the tree-shaker.
- (Ditto, if you don't wrap the type but just use
IconData const instances like we do in material/cupertino. Then the package containing IconData is responsible for tree-shaking.)
- Letting the owner of
IconDataBrands do the tree shaking:
- If you put the tree-shaking annotation on
IconDataBrands and don't let them contain const instances of IconData but just construct them at runtime, then the font_awesome_flutter package should contain a package-specific tree-shaker.
By using subtyping across two packages, it becomes ambiguous who is responsible for tree shaking. This is visible in the current issues that if you implement IconData but you provide getters for the expected fields instead of fields with const values, the tree-shaker will bail out. And this cannot be enforced from IconData to its implementers. Therefore, it would be better to lock IconData so it cannot be implemented. A similar argument can be made about extension: e.g. if you extend something, you might also want specialize the tree-shaking logic. But since the package defining the super type is responsible for tree-shaking the assets already, you can't add your specialization.
So, basically I'm proposing that when we give packages the power to do tree-shaking, only one package may be responsible for tree-shaking a single asset. Which package is responsible is determined by in which package the class definition is. And the type hierarchy of such definition should be contained within that package.
The class
IconDatahas a special constraints in its use due to the Flutter Icon Tree Shaker.The class:
flutter/packages/flutter/lib/src/widgets/icon_data.dart
Lines 22 to 53 in 995ea4d
The tree-shaker expects const instances with 3 specific fields with specific types:
flutter/packages/flutter_tools/lib/src/build_system/targets/icon_tree_shaker.dart
Lines 335 to 347 in 995ea4d
However, the class is not marked
final. Which makes it possible to subtypeIconDatain unexpected ways causing the tree-shaker to fail.Giving packages the capability to tree-shake assets
We're working on a generalized mechanism of the icon tree shaker and underlying const finder that should give this kind of powers to packages. And in order for the generalized mechanism to be simpler to reason about we'd want to enforce users recording const instances to avoid having to deal with arbitrary subtypes of their classes marked for const-collection.
We can think of packages depending on each other as layering, and for each type marked for const-collection, it should be clear in which layer (package) the assets belonging to such type should be tree-shaken. In other words, which link hook will get the information about the recorded usages.
For the purposes of tree-shaking, layering should make it exactly clear which layer (package) is doing the tree shaking. Tree-shaking in either layer is fine:
IconData,package:flutter(widgets.dart), do the tree shaking:const IconDataBrandinstances contain aconst IconDatainstance, and the tree-shaking annotations are onIconData, then the package containing the definition ofIconDatais responsible for invoking the tree-shaker.IconDataconst instances like we do in material/cupertino. Then the package containingIconDatais responsible for tree-shaking.)IconDataBrandsdo the tree shaking:IconDataBrandsand don't let them contain const instances ofIconDatabut just construct them at runtime, then thefont_awesome_flutterpackage should contain a package-specific tree-shaker.By using subtyping across two packages, it becomes ambiguous who is responsible for tree shaking. This is visible in the current issues that if you implement
IconDatabut you provide getters for the expected fields instead of fields with const values, the tree-shaker will bail out. And this cannot be enforced fromIconDatato its implementers. Therefore, it would be better to lockIconDataso it cannot be implemented. A similar argument can be made about extension: e.g. if you extend something, you might also want specialize the tree-shaking logic. But since the package defining the super type is responsible for tree-shaking the assets already, you can't add your specialization.So, basically I'm proposing that when we give packages the power to do tree-shaking, only one package may be responsible for tree-shaking a single asset. Which package is responsible is determined by in which package the class definition is. And the type hierarchy of such definition should be contained within that package.