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

[native_assets_cli] Helper to add all files in a directory as DataAssets #1346

Closed
dcharkes opened this issue Jul 16, 2024 · 2 comments · Fixed by #2097
Closed

[native_assets_cli] Helper to add all files in a directory as DataAssets #1346

dcharkes opened this issue Jul 16, 2024 · 2 comments · Fixed by #2097
Labels
P2 A bug or feature request we're likely to work on package:native_assets_cli

Comments

@dcharkes
Copy link
Collaborator

Adding all files in a directory as data assets seems to be a common use case.

In Flutter, all files in a directory can be added as assets easily:

https://docs.flutter.dev/ui/assets/assets-and-images#specifying-assets

flutter:
  assets:
    - directory/
    - directory/subdirectory/

Using these assets in Flutter is via the full file path:

https://docs.flutter.dev/ui/assets/assets-and-images#loading-text-assets

await rootBundle.loadString('assets/config.json')

We should be able to support this type of use case:

void main(List<String> args) async {
  await build(args, (config, output) async {
    await output.addDataAssetsDirectory('directory/', config);
    await output.addDataAssetsDirectory('directory/subdirectory/', config);
  });
}

Implementation sketch:

extension on BuildOutput {
  /// Adds all the files in [directory] as [DataAsset]s to this [BuildOutput].
  ///
  /// The [directory] must end with a slash and be relative to
  /// [BuildConfig.packageRoot].
  Future<void> addDataAssetsDirectory(
    String directory, {
    required BuildConfig config,
    String? linkInPackage,
  }) async {
    final packageName = config.packageName;
    final packageRoot = config.packageRoot;
    final assetDirectory = Directory.fromUri(packageRoot.resolve('assets/'));
    addDependency(assetDirectory.uri);

    await for (final assetFile in assetDirectory.list()) {
      if (assetFile is! File) {
        continue;
      }
      final assetUri = assetFile.uri;

      // The file path relative to the package root, with forward slashes.
      final name = assetUri
          .toFilePath(windows: false)
          .substring(packageRoot.toFilePath(windows: false).length);

      addAsset(
        DataAsset(
          package: packageName,
          name: name,
          file: assetUri,
        ),
        linkInPackage: linkInPackage,
      );
      addDependency(assetUri);
    }
  }
}

With such an implementation we'd also establish a convention that the a data asset in assets/foo.json will be available at runtime as package:my_package/assets/foo.json which aligns with the Flutter assets approach of making assets available via their full path.

Inspired by #1345 (comment).

Open to alternative suggestions, WDYT @mosuem?

@dcharkes dcharkes added P2 A bug or feature request we're likely to work on package:native_assets_cli labels Jul 16, 2024
@mosuem
Copy link
Member

mosuem commented Jul 16, 2024

As discussed earlier:

In would be even nicer if adding all assets was enabled by default. This requires us to land automatic treeshaking of assets first, and needs special care because of existing asset folders.

@dcharkes
Copy link
Collaborator Author

And we could add bool recursive param to the helper function.

auto-submit bot pushed a commit that referenced this issue Jul 17, 2024
Does not yet create a helper function as suggested in #1346, but does standardize the asset names to be the file path.

@mosuem I don't believe this should break https://dart-review.googlesource.com/c/sdk/+/351140, as the asset id is part of the `.dart.debug` files and I updated it there.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 A bug or feature request we're likely to work on package:native_assets_cli
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants