FlutterGen / flutter_gen

The Flutter code generator for your assets, fonts, colors, … — Get rid of all String-based APIs.
https://pub.dev/packages/flutter_gen
MIT License
1.44k stars 142 forks source link

[BUG]: The path of assets is not correct #322

Closed jinaiyuanbaojie closed 1 year ago

jinaiyuanbaojie commented 1 year ago

Is there an existing issue for this?

Version

No response

Command type

Dart command

What happened?

The plugin of svg, lottie, flare and rive generate the wrong path of the related assets. I have raised a PR https://github.com/FlutterGen/flutter_gen/pull/321. Not sure if it is a bug. If I am wrong, I think we can add a feature to show the correct path of assets under packages. :)

Relevant a pubspec.yaml.

No response

Relevant log output

No response

Code of Conduct

wasabeef commented 1 year ago

@jinaiyuanbaojie Thank you!

wasabeef commented 1 year ago

@jinaiyuanbaojie I just released v5.0.3.

noinskit commented 1 year ago

This just broke svg images in my app... path that was previously correctly packages/package_name/images/misc/foo.svg after upgrade became packages/package_name/packages/package_name/images/misc/foo.svg which obviously doesn't work. What am I doing wrong? In any case, 5.0.3 was definitely not a backward-compatible update.

wasabeef commented 1 year ago

@noinskit I implemented it with the intention of returning packages/package_name/images/misc/foo.svg, but is there a bug? 🤔

noinskit commented 1 year ago

From my perspective - yes, a bug. It worked for me before the change, stopped working after the change.

jinaiyuanbaojie commented 1 year ago

This just broke svg images in my app... path that was previously correctly packages/package_name/images/misc/foo.svg after upgrade became packages/package_name/packages/package_name/images/misc/foo.svg which obviously doesn't work. What am I doing wrong? In any case, 5.0.3 was definitely not a backward-compatible update.

Hi man, So you add the prefix packages/package_name by yourself before. Am I right? Could you post your codes here?

I am sure that the previous versions just returned the svg path as images/misc/food.svg style.

noinskit commented 1 year ago

I can't share the source directly, but what I'm doing is:

SvgPicture.asset(
  Assets.images.misc.foo.path,
  package: packageName,
)

This is in the dependent package, in a class to be used by the top-level package. I'm not manually concatenating any string prefixes. The concatenation based on package and path is done in the SvgPicture class. If I use the SvgGenImage.svg() it's working.

In 5.0.3 it's actually not possible to get the _assetName without the package prefix anymore.

Note that AssetGenImage is totally different than SvgGenImage: it has a public path property (without the package prefix) and a keyName property (with the prefix).

jinaiyuanbaojie commented 1 year ago

I can't share the source directly, but what I'm doing is:

SvgPicture.asset(
  Assets.images.misc.foo.path,
  package: packageName,
)

This is in the dependent package, in a class to be used by the top-level package. I'm not manually concatenating any string prefixes. The concatenation based on package and path is done in the SvgPicture class. If I use the SvgGenImage.svg() it's working.

In 5.0.3 it's actually not possible to get the _assetName without the package prefix anymore.

Note that AssetGenImage is totally different than SvgGenImage: it has a public path property (without the package prefix) and a keyName property (with the prefix).

Got it.

Could you try one of them? I prefer the first solution.

noinskit commented 1 year ago

@jinaiyuanbaojie I'm not sure if I understand. There are workarounds, like downgrading to 5.0.2 or avoiding SvgGenImage.path. It doesn't really change the nature of the bug.

jinaiyuanbaojie commented 1 year ago

@jinaiyuanbaojie I'm not sure if I understand. There are workarounds, like downgrading to 5.0.2 or avoiding SvgGenImage.path. It doesn't really change the nature of the bug.

Yeah man. What matters is if it is a bug. From my view, the path of assets should contains the package prefix, the absolute one.

wasabeef commented 1 year ago

@noinskit @jinaiyuanbaojie We think this change is inherently correct, since the package prefix is included only if package_parameter_enabled is enabled. However, I needed to be more specific about this intent in our CHANGELOG.

wasabeef commented 1 year ago

@noinskit @jinaiyuanbaojie What do you think about doing the following in v5.1.0 (next version) just like asset.gen.dart?

When package_parameter_enabled = true

String get path => _assetName;

String get keyName => 'packages/$packageName/\$_assetName';

When package_parameter_enabled = false

String get path => _assetName;

String get keyName => _assetName;
noinskit commented 1 year ago

@wasabeef sure, this would work for me.

jinaiyuanbaojie commented 1 year ago

@noinskit @jinaiyuanbaojie What do you think about doing the following in v5.1.0 (next version) just like asset.gen.dart?

When package_parameter_enabled = true

String get path => _assetName;

String get keyName => 'packages/$packageName/\$_assetName';

When package_parameter_enabled = false

String get path => _assetName;

String get keyName => _assetName;

@wasabeef @noinskit Yeah. That's fine:)

wasabeef commented 1 year ago

@jinaiyuanbaojie @noinskit Thank you for always supporting me. I just released the v5.1.0.

jinaiyuanbaojie commented 1 year ago

@jinaiyuanbaojie @noinskit Thank you for always supporting me. I just released the v5.1.0.

welcome. let's rock