Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

☂️ Implement pluggable transformation (via packages) of assets at build time #143348

Open
13 tasks done
andrewkolos opened this issue Feb 13, 2024 · 7 comments
Open
13 tasks done
Assignees
Labels
P2 Important issues not at the top of the work list team-tool Owned by Flutter Tool team triaged-tool Triaged by Flutter Tool team

Comments

@andrewkolos
Copy link
Contributor

andrewkolos commented Feb 13, 2024

Update. This feature is fully-implemented and available in the master branch.

Pending official documentation: flutter/website#10471
Pending sample project, which you can use to play with this feature: flutter/samples#2267


This tracks the implementation of the feature requested in #101077. The rest of this issue assumes the reader has a basic understanding of application assets in Flutter; see https://docs.flutter.dev/ui/assets/assets-and-images for more.

This feature allows users to configure their assets to be transformed by Dart CLI programs.

When declaring an asset in their pubspec, Flutter app developers can specify a transformer along with it. When building the app, the flutter CLI tool will run the transformer against the original asset file, with the output routed to the appropriate directory.

Example use case. Consider a scenario where a user wants to use the vector_graphics package to render SVG images in their app. To make an SVG file renderable by vector_graphics the user would normally have to manually run the vector_graphics_compiler package against it. The user would then need to have this transformed file bundled into their app (not the original). However, they can now instead do this:

# pubspec.yaml
...
flutter:
...
  assets:
    - path: assets/icon.svg
      transformers:
        - package: vector_graphics_compiler

Then, in their app code, the developer can the vector_graphics package to render the asset.

import 'package:vector_graphics/vector_graphics.dart';
// ...
final Widget icon = VectorGraphic(loader: AssetBytesLoader('assets/icon.svg'))

This makes adding SVGs to a Flutter app simpler. While the flutter_svg package could already render SVGs without requiring precompilation, it is significantly slower since it needs to decode and parse XML (amongst other reasons). With this feature, developers can enjoy convenience similar to that of using flutter_svg while also enjoying the superior rendering performance of vector_graphics.

vector_graphics_compiler can be used as an asset transformer because it is a CLI Dart application that accepts an --input argument (the path to the original SVG file) and an --output argument (where the output the file that can be rendered by the vector_graphics package. Any Dart CLI application that accepts these arguments and writes an output file to the provided --output path can be used as an asset transformer.

Tasks:

Cleanup tasks:

@andrewkolos andrewkolos added P2 Important issues not at the top of the work list team-tool Owned by Flutter Tool team triaged-tool Triaged by Flutter Tool team labels Feb 13, 2024
@andrewkolos andrewkolos self-assigned this Feb 13, 2024
@andrewkolos andrewkolos changed the title ☂️ Implement pluggable transformation of assets at build time using packages ☂️ Implement pluggable transformation (via packages) of assets at build time Feb 13, 2024
auto-submit bot pushed a commit that referenced this issue Feb 14, 2024
In service of #143348.

**Issue.** The `equals` implementation of `AssetsEntry` is incorrect. It compares `flavors` lists using reference equality. This PR addresses this.

This also adds a test to make sure valid asset `flavors` declarations are parsed correctly.

While we are here, this PR also includes a couple of refactorings:
  * `flutter_manifest_test.dart` is a bit large. To better match our style guide, I've factored out some related tests into their own file.
  *  A couple of changes to the `_validateListType` function in `flutter_manifest.dart`:
      * The function now returns a list of errors instead of accepting a list to append onto. This is more readable and also allows callers to know which errors were found by the call.
      * The function is renamed to `_validateList` and now accepts an `Object?` instead of an `YamlList`. If the argument is null, an appropriate error message is contained in the output. This saves callers that are only interested in validation from having to write their own null-check, which they all did before.
      * Some error strings were tweaked for increased readability and/or grammatical correctness.
auto-submit bot pushed a commit that referenced this issue Feb 16, 2024
In service of #143348.

This PR enables parsing of the pubspec yaml schemes for assets with transformations as described in #143348.
@christopherfujino
Copy link
Member

One way we could integration test this is by adding https://github.com/andrewkolos/asset_transformers_test to the github.com/flutter/tests registry (this solves needing to check in test assets to flutter/flutter). If it turns out to be more trouble than its worth, we can easily delete it.

auto-submit bot pushed a commit that referenced this issue Feb 23, 2024
…dows, MacOS, Linux, and web (also `flutter run` without hot reload support) (#143815)

See title. These are are the platforms that use the `CopyAssets` `Target` as part of their build target.

Partial implementation of #143348.
auto-submit bot pushed a commit that referenced this issue Feb 23, 2024
In service of #143348

This will make testing of asset transformation hot reload behavior easier (specifically in that we can avoid having to write a test that looks like this: https://github.com/flutter/flutter/blob/5d02c27248d9ebb0db79547f3ed0fce8060ff041/packages/flutter_tools/test/general.shard/build_system/targets/assets_test.dart#L167-L249
auto-submit bot pushed a commit that referenced this issue Mar 5, 2024
… Web) (#144161)

Partial implementation of #143348

This enables asset transformation during hot reload (except for web, because that has its own implementation of `DevFS` �). Asset transformers will be reapplied after changing any asset and performing a hot reload during `flutter run`.
@dcharkes
Copy link
Contributor

dcharkes commented Mar 7, 2024

Drive by comment.

We might be trying to solve similar use cases in the work on "native assets" (#129757), data assets (dart-lang/native#154), and hook/link.dart (dart-lang/native#153). Maybe we're trying to solve different use cases, so let's elaborate a bit.

Some of our uses cases:

  • For package messages: we'd have a translation files that will be pruned by a hook/link.dart to only include keys that are reachable in Dart code after AOT compilation with the "const finder".
  • For package intl: We have a Rust lib compiled to a static library, and link.dart will use the reachable symbols (again AOT & const finder) to use the linker to slim down the native code to a dylib that only contains the native code reachable from Dart.

The transformers in our use cases (hook/link.dart) are mostly about shrinking assets. And the goal is that any package can define a hook/link.dart.

Note that both of these use cases need to work in Dart standalone and Flutter. So unfortunately a Flutter-specific solution for resource transformation doesn't work.

We should maybe sync up on what work overlaps so we can leverage each others' work. cc @mosuem

Detailed comments:

Users author Dart packages that transform a file. These packages follow two rules: 1) they accept (the location of) an input file via an --input option, and 2) they write an output file the path provided by an --output option.

@andrewkolos Where do the entry points for these transformation scripts live? bin/?
Having a predefined CLI sounds like a hook/ (dart-lang/sdk#54334). In our work with with native assets we considered using the bin/ directory for these. However, since these hooks are not necessarily meant to be invoked by users of the package, but rather by flutter_tools and Dart tooling, we have reserved a new directory for such use cases.

When declaring an asset in their pubspec, Flutter app developers can specify a transformer along with it.

@mosuem This sound rather similar to in our proposal having hook/build.dart outputting BuildOutput.addAssetForLinking(String targetPackage, Asset asset). And then getting that assets as input LinkInput.assets in hook/link.dart. The package defining the asset specifies the linker/shrinker/compiler to use for that asset.

When building the app, the flutter CLI tool will run the transformer against the original asset file, with the output routed to the appropriate directory.

This is precisely what flutter_tools does with the dylibs output by hook/build.dart. And we'd envision flutter_tools (and the Dart SDK) something similar for "data assets" after shrinking/linking.
@andrewkolos Are you envisioning the shrunken/compiled assets to be available as the normal asset bundle in Flutter?

(Side note: hook/build.dart is currently build.dart and still have to be renamed.)

auto-submit bot pushed a commit that referenced this issue Mar 7, 2024
…er test` (#144734)

Partial implementation of #143348.

The title says it all. Feel free to experiment with the feature using this project: https://github.com/andrewkolos/asset_transformers_test.
@andrewkolos
Copy link
Contributor Author

Users author Dart packages that transform a file. These packages follow two rules: 1) they accept (the location of) an input file via an --input option, and 2) they write an output file the path provided by an --output option.

@andrewkolos Where do the entry points for these transformation scripts live? bin/? Having a predefined CLI sounds like a hook/ (dart-lang/sdk#54334). In our work with with native assets we considered using the bin/ directory for these. However, since these hooks are not necessarily meant to be invoked by users of the package, but rather by flutter_tools and Dart tooling, we have reserved a new directory for such use cases.

Yes, they live in bin. When we invoke an asset transformer, we simply run dart run <package name>, which will invoke <package dir>/bin/<package_name>.dart (docs for this behavior). I chose this for simplicity. I am also not aware of any way to invoke a program from a pub package.

When building the app, the flutter CLI tool will run the transformer against the original asset file, with the output routed to the appropriate directory.

This is precisely what flutter_tools does with the dylibs output by hook/build.dart. And we'd envision flutter_tools (and the Dart SDK) something similar for "data assets" after shrinking/linking. @andrewkolos Are you envisioning the shrunken/compiled assets to be available as the normal asset bundle in Flutter?

Yes, the file output by the dart run invocation is available for loading through the AssetBundle framework API like any other asset. For an example, see the example app (setup, dart code). These assets are only special in that they are transformed as they are copied to the device/build directory.

@dcharkes
Copy link
Contributor

Users author Dart packages that transform a file. These packages follow two rules: 1) they accept (the location of) an input file via an --input option, and 2) they write an output file the path provided by an --output option.

@andrewkolos Where do the entry points for these transformation scripts live? bin/? Having a predefined CLI sounds like a hook/ (dart-lang/sdk#54334). In our work with with native assets we considered using the bin/ directory for these. However, since these hooks are not necessarily meant to be invoked by users of the package, but rather by flutter_tools and Dart tooling, we have reserved a new directory for such use cases.

Yes, they live in bin. When we invoke an asset transformer, we simply run dart run <package name>, which will invoke <package dir>/bin/<package_name>.dart (docs for this behavior). I chose this for simplicity. I am also not aware of any way to invoke a program from a pub package.

For the native assets feature, dartdev and flutter_tools do the invocation with a dart /absolute/path/to/hook/build.dart, instead of relying on dart pub run.

When building the app, the flutter CLI tool will run the transformer against the original asset file, with the output routed to the appropriate directory.

This is precisely what flutter_tools does with the dylibs output by hook/build.dart. And we'd envision flutter_tools (and the Dart SDK) something similar for "data assets" after shrinking/linking. @andrewkolos Are you envisioning the shrunken/compiled assets to be available as the normal asset bundle in Flutter?

Yes, the file output by the dart run invocation is available for loading through the AssetBundle framework API like any other asset. For an example, see the example app (setup, dart code). These assets are only special in that they are transformed as they are copied to the device/build directory.

Cool! The "data assets" for Dart standalone packages such as package:intl will be available with the same API in Flutter as well. (And we need to port the API to dart: as well.)

@andrewkolos
Copy link
Contributor Author

For the native assets feature, dartdev and flutter_tools do the invocation with a dart /absolute/path/to/hook/build.dart, instead of relying on dart pub run.?

How are the dependencies of the package resolved then? For example, if I

  1. create a new flutter project,
  2. add vector_graphics_compiler as a dev dependency, and
  3. directly invoke the package via dart run Users/andrewkolos/.pub-cache/hosted/pub.dev/vector_graphics_compiler-1.1.9+2/bin/vector_graphics_compiler.dart, then

I will get compiler errors:

Error: Couldn't resolve the package 'args' in 'package:args/args.dart'.
Error: Couldn't resolve the package 'vector_graphics_compiler' in 'package:vector_graphics_compiler/src/svg/colors.dart'.
Error: Couldn't resolve the package 'vector_graphics_compiler' in 'package:vector_graphics_compiler/vector_graphics_compiler.dart'.
Error: Couldn't resolve the package 'path' in 'package:path/path.dart'.
../../.pub-cache/hosted/pub.dev/vector_graphics_compiler-1.1.9+2/bin/vector_graphics_compiler.dart:7:8: Error: Not found: 'package:args/args.dart'
import 'package:args/args.dart';
       ^
../../.pub-cache/hosted/pub.dev/vector_graphics_compiler-1.1.9+2/bin/vector_graphics_compiler.dart:8:8: Error: Not found: 'package:vector_graphics_compiler/src/svg/colors.dart'
import 'package:vector_graphics_compiler/src/svg/colors.dart';
       ^
../../.pub-cache/hosted/pub.dev/vector_graphics_compiler-1.1.9+2/bin/vector_graphics_compiler.dart:9:8: Error: Not found: 'package:vector_graphics_compiler/vector_graphics_compiler.dart'
import 'package:vector_graphics_compiler/vector_graphics_compiler.dart';
       ^
../../.pub-cache/hosted/pub.dev/vector_graphics_compiler-1.1.9+2/bin/util/isolate_processor.dart:10:8: Error: Not found: 'package:vector_graphics_compiler/vector_graphics_compiler.dart'
import 'package:vector_graphics_compiler/vector_graphics_compiler.dart';
       ^
Error: Couldn't resolve the package 'vector_graphics_compiler' in 'package:vector_graphics_compiler/src/debug_format.dart'.
../../.pub-cache/hosted/pub.dev/vector_graphics_compiler-1.1.9+2/bin/vector_graphics_compiler.dart:12:8: Error: Not found: 'package:path/path.dart'
import 'package:path/path.dart' as p;

@dcharkes
Copy link
Contributor

How are the dependencies of the package resolved then?

The packages_config.json for the root package is used.

(We've had a bunch of conversations whether we should allow hooks of different packages to resolve their dependencies to different versions, but decided that that can have negative consequences. Especially if the code in src/ in the root package imports package:a and uses its Dart API, and passes an asset that is transformed by package:a in a hook, the author of package:a probably wants to rely on that being the same version of package:a. See dart-lang/pub#3794 and dart-lang/native#160 for some more considerations.)

flutter_tools and dartdev take care of passing the package config to the dart invocation. For example:

final PackageConfig packageConfig = await loadPackageConfigWithLogging(
packagesFile,
logger: environment.logger,
);
final NativeAssetsBuildRunner buildRunner = _buildRunner ??
NativeAssetsBuildRunnerImpl(
projectUri,
packageConfig,
fileSystem,
environment.logger,
);

auto-submit bot pushed a commit that referenced this issue Mar 11, 2024
)

In service of #143348

When invoking a package to transform an asset, we set `FLUTTER_BUILD_MODE` to the CLI name of the build mode being used. Inspired by #101077 (comment):

> Do transformers know whether they get executed in debug or release mode? I kinda imagine that being useful. Ex: There's a transformer that optimizes the file size of images. Depending on the amount and size of the images, that could take a significant amount of time. Therefore, I might want to only execute it in release builds.

Note for the reviewer: the interesting part of this change can be found in the commit [set environment variable to build mode when running asset transformer�](579912d). The rest of the change is updating call sites with a new argument.
auto-submit bot added a commit that referenced this issue Mar 11, 2024
…ses (#144752)" (#144957)

Reverts: #144752
Initiated by: andrewkolos
Reason for reverting: compilation issue has turned the tree red
Original PR Author: andrewkolos

Reviewed By: {christopherfujino}

This change reverts the following previous change:
In service of #143348

When invoking a package to transform an asset, we set `FLUTTER_BUILD_MODE` to the CLI name of the build mode being used. Inspired by #101077 (comment):

> Do transformers know whether they get executed in debug or release mode? I kinda imagine that being useful. Ex: There's a transformer that optimizes the file size of images. Depending on the amount and size of the images, that could take a significant amount of time. Therefore, I might want to only execute it in release builds.

Note for the reviewer: the interesting part of this change can be found in the commit [set environment variable to build mode when running asset transformer…](579912d). The rest of the change is updating call sites with a new argument.
@christopherfujino
Copy link
Member

christopherfujino commented Mar 22, 2024

A few thoughts:

  1. before we ship, we should add a printTrace to the tool about each asset transformer we're invoking and the path/arguments we called it with (edit by @andrewkolos: done in print traces when transforming an asset #146374)
  2. not a blocker, but we should consider in non-verbose mode adding the time asset transformers took to the durations we print out about how long the hot reload took (e.g. Reloaded 0 libraries in 17,285ms (compile: 28 ms, reload: 0 ms, reassemble: 54 ms).). Consider just adding a stopwatch for how long processing the asset bundle took.
  3. not a blocker, but we should add analytics around this feature.

auto-submit bot pushed a commit that referenced this issue Mar 25, 2024
…144660)

In service of #143348

This PR makes hot reloads reflect changes to transformer configurations under the `assets` section in pubspec.yaml.
This PR is optimized for ease of implementation, and should not be merged as-is. If it is merged as-is, seriously consider creating a tech debt issue and assigning it to someone to make sure it gets resolved.
auto-submit bot pushed a commit that referenced this issue Apr 22, 2024
From #143348 (comment):

> before we ship, we should add a printTrace to the tool about each asset transformer we're invoking and the path/arguments we called it with

I think this is a good idea since asset transformers can be arbitrary Dart programs�meaning that a lot can go wrong when running them. For example, they can hang indefinitely or perform some sort of I/O that later results in a tool crash. Knowing that asset transformation was involved when debugging a crash (or a slow/stuck `flutter build`) could be useful, so I think adding a `printTrace` or two is a good idea (or at least not a bad one).
gilnobrega pushed a commit to gilnobrega/flutter that referenced this issue Apr 22, 2024
From flutter#143348 (comment):

> before we ship, we should add a printTrace to the tool about each asset transformer we're invoking and the path/arguments we called it with

I think this is a good idea since asset transformers can be arbitrary Dart programs�meaning that a lot can go wrong when running them. For example, they can hang indefinitely or perform some sort of I/O that later results in a tool crash. Knowing that asset transformation was involved when debugging a crash (or a slow/stuck `flutter build`) could be useful, so I think adding a `printTrace` or two is a good idea (or at least not a bad one).
auto-submit bot pushed a commit that referenced this issue Apr 26, 2024
In service of #143348

This adds a simple integration test for the new asset transformation feature.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 Important issues not at the top of the work list team-tool Owned by Flutter Tool team triaged-tool Triaged by Flutter Tool team
Projects
None yet
Development

No branches or pull requests

3 participants