BirjuVachhani / spider

A small dart library to generate Assets dart code from assets folder.
https://spider.birju.dev/
Apache License 2.0
190 stars 19 forks source link

Ability to use generator with assets bundled with package #64

Closed Aqluse closed 1 year ago

Aqluse commented 2 years ago

While making a package with assets bundled with it, I encountered a problem that spider cannot recognize package related path in yaml config and then cannot generate proper classes for assets following package convention paths.

https://docs.flutter.dev/development/ui/assets-and-images#bundling-of-package-assets

Manual output edits make it work.

I would suggest to make a tool consider paths like 'packages/some_package_name' internally as 'lib' when assuring its existence and then just use the path entered as it is. Also I would added a flag to drop this path prefix as some potential components may allow to put the package prefix separately from path, so it won't double in for the case.

BirjuVachhani commented 2 years ago

@Aqluse So you want this tool to support asset ref generation for package assets like this package?

Aqluse commented 2 years ago

@BirjuVachhani I can't even call it a package since I won't be able to use the images out-of-box without doing a manual copy of it to my directories.

But If we would imagine that the package distributes some images as paths or within widgets from the package, we would have to manually copy flags to our directory at exactly the same relative path that is given within plain paths/exported widget that use the flags, so paths are given to that widgets.

Aqluse commented 2 years ago

There's a difference between assets inside the /lib directory and outside.

Assets that are outside could only be used (without error) inside the package because it getting included for it.

Assets that are inside /lib that addressed in proper (package-relative) way and enclosed to pubspec.yaml in proper the same way (and also separately, you cannot enlist them all by typing directory path) could be used outside of actual package without copying assets itself and enlisting in pubspec.yaml again. That includes fonts (has its own schema for pubspec) and any kind of files, usually icons or sounds, but not limited.

There's a complicated private API of resolving these paths by AssetBundle you can refer, but I simplified it a bit in PR.

P.S: Also you can look into compiled package to see that nothing ouside of /lib is getting included to package, that's why all of that happening.

BirjuVachhani commented 2 years ago

That's wonderful but how do I test your PR? I have never used this flags package. https://github.com/BirjuVachhani/spider/pull/65#issuecomment-1221332954

BirjuVachhani commented 2 years ago

Assets that are outside could only be used (without error) inside the package because it getting included for it.

This flags package seems to have assets outside /lib directory. Does this mean it won't work with your PR? If it can work, then would you please mind explaining how?

Aqluse commented 2 years ago

You just need to put package-assets at path like '/lib/anynameforassetsdir/anyfilename.anyfileextension'. Any depth will satisfy, even assets directly in a root of /lib.

Additionally you should add every package-asset separately in pubspec assets with paths like 'packages/anynameforassetsdir/anyfilename.anyfileextension' - the same as paths that will be generated and could be correctly referred.

Any package that satisfy these two rules will be applicable.

By putting in a /lib you ensure that asset will be resolved by package user correctly (that it will be actually present and holds a permanent path). By putting path in a pubspec you ensure that asset will be resolved by AssetBundle, thus available to reference from its resources. Putting separately in a pubspec is just a requirement.

BirjuVachhani commented 2 years ago

You just need to put package-assets at path like '/lib/anynameforassetsdir/anyfilename.anyfileextension'. Any depth will satisfy, even assets directly in a root of /lib.

Did you mean copying all the assets from this package and put it in the /lib directory?

Aqluse commented 1 year ago

Exactly. For the library package which code will be used and for which codegen will be used. If completed successfully, you will get library with assets accessible when library components are used outside of it or valid paths if codegen output will be exported from library.

BirjuVachhani commented 1 year ago

If I understand this correctly then this is the scenario:

I have a package X with 5 images inside its lib/images directory. I wanna use this spider tool to generate references for these images in my package so that the user of the package can use those generated dart references to point to its assets if they want to.

Package Side Configuration

Package Structure:

lib/images
├── image1.png
├── image2.png
├── image3.png
├── image4.png
└── image5.png

spider.yaml file configuration in the package:

groups:
  - path: packages/<package_name>/images
    class_name: PackageImages
    types: [ .png, .jpg, .jpeg, .webp, .webm, .bmp ]

Running spider build command would generate this:

lib/resources
├── package_images.dart
└── resources.dart
part of 'resources.dart';

class PackageImages {
  PackageImages._();

  static const String image1 = 'packages/<package_name>/images/image1.png';
  static const String image2 = 'packages/<package_name>/images/image2.png';
  static const String image3 = 'packages/<package_name>/images/image3.png';
  static const String image4 = 'packages/<package_name>/images/image4.png';
  static const String image5 = 'packages/<package_name>/images/image5.png';
}

This generated PackageImages can be used by the package itself internally and external applications that depend on this package also refer to this if it is exported.

Application Configuration

Application's pubspec.yaml:

flutter:
  assets:
    - packages/<package_name>/images/image1.png
    - packages/<package_name>/images/image2.png
    - packages/<package_name>/images/image3.png
    - packages/<package_name>/images/image4.png
    - packages/<package_name>/images/image5.png

It is mendatory for the application to specify each and every asset path individually. Otherwise AssetBundle won't be able to find the package assets. So specifying packages/<package_name>/images/ doesn't work.

Application code can now use PackageImages:

Image.asset(PackageImages.image1);

I never knew flutter could do this! I tested it myself with your PR and it works!

@Aqluse Is my understanding of what you implemented correct here?

Also, For this package country_icons, all the assets are put into icons/flags directory which is outside lib directory of the package. How does your PR generate assets for this scenario? What would be the spider configuration of for the application project (since its a third party package and it is not using spider, it won't have asset references generated inside it)?

Aqluse commented 1 year ago

It is mendatory for the application to specify each and every asset path individually. Otherwise AssetBundle won't be able to find the package assets. So specifying packages//images/ doesn't work.

It is mendatory for the package to specify each and every asset path individually. Otherwise AssetBundle won't be able to find the package assets. So specifying packages//images/ doesn't work. User will be warned that it should be specified individually automatically when compiling (if I remember right). User will get runtime error if any used reference is not enlisted at pubspec, as usual, for non-package assets too.

Anything you write above seems correct.

Aqluse commented 1 year ago

Also, For this package country_icons, all the assets are put into icons/flags directory which is outside lib directory of the package. How does your PR generate assets for this scenario?

My PR doesn't handle this scenario. It can be handled as the spider does at the moment, so you will have to do a fork of country_icons package and put it right inside your application. Or fork and move icons directory to /lib, after it my PR will be useful.

it won't have asset references generated inside it

Unfortunately it won't be, until merged by a maintainer of that package.

BirjuVachhani commented 1 year ago

Got it. I understand this now. Thanks @Aqluse for the PR.

BirjuVachhani commented 1 year ago

PR is merged. Will be included in the next release. Thanks.

BirjuVachhani commented 1 year ago

Available in v4.1.0