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

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

Closed
2 tasks done
jinaiyuanbaojie opened this issue Oct 18, 2022 · 16 comments · Fixed by #330 or #331
Closed
2 tasks done

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

jinaiyuanbaojie opened this issue Oct 18, 2022 · 16 comments · Fixed by #330 or #331
Labels
bug Something isn't working
Milestone

Comments

@jinaiyuanbaojie
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

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 #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

  • I agree to follow this project's Code of Conduct
@jinaiyuanbaojie jinaiyuanbaojie added the bug Something isn't working label Oct 18, 2022
@jinaiyuanbaojie jinaiyuanbaojie changed the title [BUG]: [BUG]: The path of assets is not correct Oct 19, 2022
@wasabeef wasabeef added this to the 5.0.3 milestone Oct 20, 2022
@wasabeef
Copy link
Member

@jinaiyuanbaojie Thank you!

@wasabeef
Copy link
Member

@jinaiyuanbaojie
I just released v5.0.3.

@noinskit
Copy link

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
Copy link
Member

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

@noinskit
Copy link

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

@jinaiyuanbaojie
Copy link
Contributor Author

jinaiyuanbaojie commented Oct 24, 2022

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
Copy link

noinskit commented Oct 24, 2022

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
Copy link
Contributor Author

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.

  • Assets.images.misc.foo.svg()
SvgPicture.asset(
  Assets.images.misc.foo.path
)

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

@noinskit
Copy link

@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
Copy link
Contributor Author

@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
Copy link
Member

wasabeef commented Oct 27, 2022

@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
Copy link
Member

@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
Copy link

@wasabeef sure, this would work for me.

@jinaiyuanbaojie
Copy link
Contributor Author

@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:)

This was referenced Oct 31, 2022
@wasabeef
Copy link
Member

wasabeef commented Nov 1, 2022

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

@jinaiyuanbaojie
Copy link
Contributor Author

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

welcome. let's rock

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants