packages icon indicating copy to clipboard operation
packages copied to clipboard

[vector_graphics] Use hooks in addition to asset transformers

Open mosuem opened this issue 3 months ago • 14 comments

Requires flutter config --enable-dart-data-assets.

This is an example of how replacing asset transformers with build/link hooks could look like. The basic idea is that package vector_graphics_compiler exposes a helper method, here compileSvg, which is then called in the package's build hook. Options can be set there as well.

For now, asset transformers and build hooks can simply co-exist - in the future, when hooks are stable enough, we will want to deprecate asset transformers and move towards this mechanism.

mosuem avatar Sep 01 '25 11:09 mosuem

Any progress on the presubmit failures?

chinmaygarde avatar Sep 22 '25 18:09 chinmaygarde

Slow progress :) There are a lot of presubmits..

mosuem avatar Sep 23 '25 06:09 mosuem

@chinmaygarde How to solve the remaining failures?

The legacy N-1 will fail as this is a feature only present on Flutter main branch. For the other failures, I don't understand how to read what is failing. They are quite verbose. Help would be appreciated!

mosuem avatar Sep 23 '25 11:09 mosuem

This PR needs to re-target the master branch. It cannot land on main.

jtmcdole avatar Sep 23 '25 14:09 jtmcdole

This PR needs to re-target the master branch. It cannot land on main.

flutter/packages doesn't have master.

stuartmorgan-g avatar Sep 24 '25 14:09 stuartmorgan-g

The legacy N-1 will fail as this is a feature only present on Flutter main branch.

N-1 and N-2 tests only run if the package's pubspec.yaml claims to support that version of the SDK. If this PR makes a package not work with older versions of the SDK, then it's an error to land it without changing the SDK requirement, as that would break clients using older versions of Flutter.

stuartmorgan-g avatar Sep 24 '25 14:09 stuartmorgan-g

For the other failures, I don't understand how to read what is failing. They are quite verbose. Help would be appreciated!

A bunch of them appear to be flake. At least these two appear to be real though:

  • https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8702950596237063569/+/u/Run_package_tests/build_examples/stdout
    The output contained unsupported output:
    - Asset with type "data_assets/data" is not a supported asset type (code_assets/code are supported)
    
  • https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8702950595889471601/+/u/Run_package_tests/build_examples__Win32_/stdout
    Target dart_build failed : error : Building native assets failed. See the logs for more details.
    

For the second, I'm not sure what "the logs" that flutter error message is referring to are.

stuartmorgan-g avatar Sep 24 '25 14:09 stuartmorgan-g

For the other failures, I don't understand how to read what is failing. They are quite verbose. Help would be appreciated!

A bunch of them appear to be flake. At least these two appear to be real though:

  • https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8702950596237063569/+/u/Run_package_tests/build_examples/stdout
    The output contained unsupported output:
    - Asset with type "data_assets/data" is not a supported asset type (code_assets/code are supported)
    
  • https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8702950595889471601/+/u/Run_package_tests/build_examples__Win32_/stdout
    Target dart_build failed : error : Building native assets failed. See the logs for more details.
    

For the second, I'm not sure what "the logs" that flutter error message is referring to are.

Thanks! This is probably due to the fact that the data-assets feature is not on stable yet. How do I only run the tests against dev?

mosuem avatar Sep 29 '25 14:09 mosuem

Also, I did upgrade the minimum SDK version, but the N-1 and N-2 tests are still running and failing. Or did I misunderstand something?

mosuem avatar Sep 29 '25 14:09 mosuem

Thanks! This is probably due to the fact that the data-assets feature is not on stable yet. How do I only run the tests against dev?

Generally speaking, you don't. Is there a specific need for landing this PR prior to stable support that requires overriding our standard policy?

Also, I did upgrade the minimum SDK version, but the N-1 and N-2 tests are still running and failing.

These lines are breaking things. It looks like those are something from the original repo's configuration that I didn't notice and clean up when importing, and they should be removed.

stuartmorgan-g avatar Sep 29 '25 15:09 stuartmorgan-g

Thanks! This is probably due to the fact that the data-assets feature is not on stable yet. How do I only run the tests against dev?

Generally speaking, you don't. Is there a specific need for landing this PR prior to stable support that requires overriding our standard policy?

I would be interested in having people try out this feature easily. So a -dev version of vector_graphics_compiler could be released with a dependency on a -dev version of the SDK. Or is that overly complicated? Alternatively, I could release a new package with the functionality which could be used as a drop-in replacement, and deprecate that package sometime in the future in favor of merging into this package.

mosuem avatar Sep 29 '25 15:09 mosuem

So a -dev version of vector_graphics_compiler could be released with a dependency on a -dev version of the SDK.

I'm aware of that, but we aren't really set up to allow for that.

Or is that overly complicated?

We do not currently have a release branch system for the packages repo, so doing this would in practice prevent us from shipping any updates of any kind to current clients of vector_graphics for as long as it takes to ship a new version of Flutter stable, which is the reason for the policy.

Alternatively, I could release a new package with the functionality which could be used as a drop-in replacement, and deprecate that package sometime in the future in favor of merging into this package.

Is that really easier than having people use a git dependency pointing to a branch? The discoverability of -dev versions is low (one of the reasons we've never prioritized making shipping -dev versions from this repo a thing), and the discoverability of a new alternate package would be even lower. I would think anyone who could be told to use a new package could be told to use a canned snippet with the correct git dependency?

stuartmorgan-g avatar Sep 29 '25 15:09 stuartmorgan-g

From triage: What's the status of this PR? Was the decision to wait for the next stable release?

stuartmorgan-g avatar Nov 04 '25 19:11 stuartmorgan-g

From triage: Is this unblocked now that Flutter 3.38 is out?

stuartmorgan-g avatar Dec 09 '25 19:12 stuartmorgan-g