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.

Sign up for GitHub

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

Jump to bottom

✨ Add package to generated assets #401

Merged
merged 12 commits into from
Oct 19, 2023
Merged

✨ Add package to generated assets #401

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
064c5ff
✨ Add `package` to generated assets
AlexV525 Jun 20, 2023
3c11a10
✅ Add tests
AlexV525 Jun 21, 2023
cc0cece
✅ Fix and add tests
AlexV525 Jul 4, 2023
67f58e7
✅ Fix generate tests
AlexV525 Jul 4, 2023
dd4337c
✅ Fix separator on Windows for command tests
AlexV525 Jul 4, 2023
7e04eaf
🎨 Fix file formats
AlexV525 Jul 5, 2023
bb180a8
🎨 Try `dart_style` matching
AlexV525 Jul 5, 2023
4352ebf
♻️ Split `generatePackageNameForConfig` and add tests
AlexV525 Jul 5, 2023
1a59d65
✅ Use `@deprecated` instead of `@Deprecated`
AlexV525 Jul 5, 2023
fbad33b
📝 Add README
AlexV525 Jul 5, 2023
e879ed1
📝 ++
AlexV525 Aug 22, 2023
ca1e1e6
Merge branch 'main' into feat/add-package-to-gen
AlexV525 Oct 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,55 @@ flutter:

These configurations will generate **`assets.gen.dart`** under the **`lib/gen/`** directory by default.

#### Generate for packages

If you want to generate assets for a package,
use `package_parameter_enabled` under `flutter_gen > assets > outputs`.

```yaml
flutter_gen:
assets:
outputs:
package_parameter_enabled: false # <- Add this line.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: package_parameter_enabled seems quite long, surely package_parameter would also be acceptable?

and in your example, would you set this to "true" to have the package parameter included?

Copy link
Member Author

@AlexV525 AlexV525 Aug 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: package_parameter_enabled seems quite long, surely package_parameter would also be acceptable?

The parameter is not new, so it'd be a breaking change if we use another name.

and in your example, would you set this to "true" to have the package parameter included?

Sure. Sounds good too.

```

This would add the package constant to the generated class. For example:

```dart
class Assets {
Assets._();

static const String package = 'test';

static const $AssetsImagesGen images = $AssetsImagesGen();
static const $AssetsUnknownGen unknown = $AssetsUnknownGen();
}
```

Then you can use assets with the package implicitly or explicitly:

```dart
// Implicit usage for `Image`/`SvgPicture`/`Lottie`.
Widget build(BuildContext context) {
return Assets.images.icons.paint.svg(
width: 120,
height: 120,
);
}
```
or
```dart
// Explicit usage for `Image`/`SvgPicture`/`Lottie`.
Widget build(BuildContext context) {
return SvgPicture.asset(
Assets.images.icons.paint.path,
package: Assets.package,
width: 120,
height: 120,
);
}
```

#### Usage Example

[FlutterGen] generates [Image](https://api.flutter.dev/flutter/widgets/Image-class.html) class if the asset is Flutter supported image format.
Expand Down
22 changes: 18 additions & 4 deletions examples/example_resources/lib/gen/assets.gen.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ class $AssetsUnknownGen {
class ResAssets {
ResAssets._();

static const String package = 'example_resources';

static const $AssetsImagesGen images = $AssetsImagesGen();
static const $AssetsUnknownGen unknown = $AssetsUnknownGen();
}
Expand All @@ -64,6 +66,8 @@ class AssetGenImage {

final String _assetName;

static const String package = 'example_resources';

Image image({
Key? key,
AssetBundle? bundle,
Expand All @@ -84,7 +88,7 @@ class AssetGenImage {
bool matchTextDirection = false,
bool gaplessPlayback = false,
bool isAntiAlias = false,
String? package = 'example_resources',
@deprecated String? package = package,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No deprecate message?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because of a dart_style formatting issue and I cannot get a consist behavior when generating files with @Deprecated, so go for the @deprecated.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah sorry you did explain that back in July. Sorry for asking again!

I just tried again locally, and dart format does not change it when it's formatted it either:

    @Deprecated("short") String? package,

or

    @Deprecated("really really really really really really really really really really really long ")
    String? package,

Ok, it's such a minor thing, it's not worth debugging more. But maybe off-topic, but is the dart_style package still needed? dart format works fine without it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but is the dart_style package still needed? dart format works fine without it.

So far it looks to be required because codes are generated when running runners and produced formatted files without calling dart format.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see, dart_style is used during the generating of the code.

I noticed the version of dart_style listed in packages/core/pubspec.yaml is a year old, and the dart format being run in the GitHub actions seems a lot newer than that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed the version of dart_style listed in packages/core/pubspec.yaml is a year old, and the dart format being run in the GitHub actions seems a lot newer than that.

That's fine unless it released v3 then we'll need to manually bump it.

FilterQuality filterQuality = FilterQuality.low,
int? cacheWidth,
int? cacheHeight,
Expand Down Expand Up @@ -119,7 +123,7 @@ class AssetGenImage {

ImageProvider provider({
AssetBundle? bundle,
String? package = 'example_resources',
String? package = package,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing deprecate message?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks!

}) {
return AssetImage(
_assetName,
Expand All @@ -138,11 +142,14 @@ class SvgGenImage {

final String _assetName;

static const String package = 'example_resources';

SvgPicture svg({
Key? key,
bool matchTextDirection = false,
AssetBundle? bundle,
String? package = 'example_resources',
@Deprecated('Do not use package for a package asset')
Copy link
Contributor

@bramp bramp Oct 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the deprecated message, what should the developer do instead? "Do not use" doesn't tell them what to do instead.

@deprecated("Package is no longer needed as it now defaults to correct package") ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They should not use package for a package asset, which literally means not using it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah and it's @deprecated actually.

String? package = package,
double? width,
double? height,
BoxFit fit = BoxFit.contain,
Expand Down Expand Up @@ -191,6 +198,8 @@ class FlareGenImage {

final String _assetName;

static const String package = 'example_resources';

FlareActor flare({
String? boundsNode,
String? animation,
Expand Down Expand Up @@ -234,6 +243,8 @@ class RiveGenImage {

final String _assetName;

static const String package = 'example_resources';

RiveAnimation rive({
String? artboard,
List<String> animations = const [],
Expand Down Expand Up @@ -269,6 +280,8 @@ class LottieGenImage {

final String _assetName;

static const String package = 'example_resources';

LottieBuilder lottie({
Animation<double>? controller,
bool? animate,
Expand All @@ -287,7 +300,8 @@ class LottieGenImage {
double? height,
BoxFit? fit,
AlignmentGeometry? alignment,
String? package = 'example_resources',
@Deprecated('Do not use package for a package asset')
String? package = package,
bool? addRepaintBoundary,
FilterQuality? filterQuality,
void Function(String)? onWarning,
Expand Down
48 changes: 35 additions & 13 deletions packages/command/test/flutter_gen_command_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,41 @@
import 'dart:io' show Platform;

import 'package:flutter_gen_core/utils/version.dart';
import 'package:test/test.dart';
import 'package:test_process/test_process.dart';

final separator = Platform.pathSeparator;

void main() {
test('Execute fluttergen', () async {
final process =
await TestProcess.start('dart', ['bin/flutter_gen_command.dart']);
expect(await process.stdout.next,
equals('$flutterGenVersion Loading ... command/pubspec.yaml'));
final process = await TestProcess.start(
'dart',
['bin/flutter_gen_command.dart'],
);
expect(
await process.stdout.next,
equals('$flutterGenVersion Loading ... command${separator}pubspec.yaml'),
);
await process.shouldExit(0);
});

test('Execute fluttergen --config pubspec.yaml', () async {
var process = await TestProcess.start(
'dart', ['bin/flutter_gen_command.dart', '--config', 'pubspec.yaml']);
expect(await process.stdout.next,
equals('$flutterGenVersion Loading ... command/pubspec.yaml'));
'dart',
['bin/flutter_gen_command.dart', '--config', 'pubspec.yaml'],
);
expect(
await process.stdout.next,
equals('$flutterGenVersion Loading ... command${separator}pubspec.yaml'),
);
await process.shouldExit(0);
});

test('Execute fluttergen --help', () async {
var process = await TestProcess.start(
'dart', ['bin/flutter_gen_command.dart', '--help']);
'dart',
['bin/flutter_gen_command.dart', '--help'],
);
expect(await process.stdout.next,
equals('-c, --config Set the path of pubspec.yaml.'));
final line = await process.stdout.next;
Expand All @@ -31,18 +45,26 @@ void main() {

test('Execute fluttergen --version', () async {
var process = await TestProcess.start(
'dart', ['bin/flutter_gen_command.dart', '--version']);
'dart',
['bin/flutter_gen_command.dart', '--version'],
);
expect(await process.stdout.next, equals(flutterGenVersion));
await process.shouldExit(0);
});

test('Execute wrong argments with fluttergen --wrong', () async {
var process = await TestProcess.start(
'dart', ['bin/flutter_gen_command.dart', '--wrong']);
expect(await process.stderr.next,
equals('Could not find an option named "wrong".'));
'dart',
['bin/flutter_gen_command.dart', '--wrong'],
);
expect(
await process.stderr.next,
equals('Could not find an option named "wrong".'),
);
expect(
await process.stderr.next, equals('usage: flutter_gen [options...]'));
await process.stderr.next,
equals('usage: flutter_gen [options...]'),
);
await process.shouldExit(0);
});
}
47 changes: 39 additions & 8 deletions packages/core/lib/generators/assets_generator.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,14 @@ String generateAssets(
return formatter.format(buffer.toString());
}

String? generatePackageNameForConfig(AssetsGenConfig config) {
if (config.flutterGen.assets.outputs.packageParameterEnabled) {
return config._packageName;
} else {
return null;
}
}

List<String> _getAssetRelativePathList(
String rootPath,
List<String> assets,
Expand Down Expand Up @@ -362,8 +370,14 @@ String _dotDelimiterStyleDefinition(
assetTypeQueue.addAll(assetType.children);
}
}
buffer.writeln(_dotDelimiterStyleAssetsClassDefinition(
className, assetsStaticStatements));
final String? packageName = generatePackageNameForConfig(config);
buffer.writeln(
_dotDelimiterStyleAssetsClassDefinition(
className,
assetsStaticStatements,
packageName,
),
);
return buffer.toString();
}

Expand Down Expand Up @@ -424,27 +438,40 @@ String _flatStyleDefinition(
.whereType<_Statement>()
.toList();
final className = config.flutterGen.assets.outputs.className;
return _flatStyleAssetsClassDefinition(className, statements);
final String? packageName = generatePackageNameForConfig(config);
return _flatStyleAssetsClassDefinition(className, statements, packageName);
}

String _flatStyleAssetsClassDefinition(
String className,
List<_Statement> statements,
String? packageName,
) {
final statementsBlock =
statements.map((statement) => '''${statement.toDartDocString()}
${statement.toStaticFieldString()}
''').join('\n');
return _assetsClassDefinition(className, statements, statementsBlock);
return _assetsClassDefinition(
className,
statements,
statementsBlock,
packageName,
);
}

String _dotDelimiterStyleAssetsClassDefinition(
String className,
List<_Statement> statements,
String? packageName,
) {
final statementsBlock =
statements.map((statement) => statement.toStaticFieldString()).join('\n');
return _assetsClassDefinition(className, statements, statementsBlock);
return _assetsClassDefinition(
className,
statements,
statementsBlock,
packageName,
);
}

String _assetValuesDefinition(List<_Statement> statements) {
Expand All @@ -468,12 +495,14 @@ String _assetsClassDefinition(
String className,
List<_Statement> statements,
String statementsBlock,
String? packageName,
) {
final valuesBlock = _assetValuesDefinition(statements);
return '''
class $className {
$className._();

${packageName != null ? "\n static const String package = '$packageName';" : ''}

$statementsBlock
$valuesBlock
}
Expand Down Expand Up @@ -504,7 +533,8 @@ class $className {
}

String _assetGenImageClassDefinition(String packageName) {
final packageParameter = packageName.isNotEmpty ? " = '$packageName'" : '';
final bool isPackage = packageName.isNotEmpty;
final packageParameter = isPackage ? ' = package' : '';

final keyName = packageName.isEmpty
? '_assetName'
Expand All @@ -516,6 +546,7 @@ class AssetGenImage {
const AssetGenImage(this._assetName);

final String _assetName;
${isPackage ? "\n static const String package = '$packageName';" : ''}

Image image({
Key? key,
Expand All @@ -537,7 +568,7 @@ class AssetGenImage {
bool matchTextDirection = false,
bool gaplessPlayback = false,
bool isAntiAlias = false,
String? package$packageParameter,
${isPackage ? '@deprecated ' : ''}String? package$packageParameter,
FilterQuality filterQuality = FilterQuality.low,
int? cacheWidth,
int? cacheHeight,
Expand Down
10 changes: 5 additions & 5 deletions packages/core/lib/generators/integrations/flare_integration.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ class FlareIntegration extends Integration {
FlareIntegration(String packageParameterLiteral)
: super(packageParameterLiteral);

String? get packageExpression => packageParameterLiteral.isNotEmpty
? 'packages/$packageParameterLiteral/'
: null;
String? get packageExpression =>
isPackage ? 'packages/$packageParameterLiteral/' : null;

@override
List<String> get requiredImports => [
Expand All @@ -22,6 +21,7 @@ class FlareIntegration extends Integration {
const FlareGenImage(this._assetName);

final String _assetName;
${isPackage ? "\n static const String package = '$packageParameterLiteral';" : ''}

FlareActor flare({
String? boundsNode,
Expand All @@ -39,7 +39,7 @@ class FlareIntegration extends Integration {
bool antialias = true,
}) {
return FlareActor(
${packageExpression == null ? '_assetName' : '\'$packageExpression\$_assetName\''},
${isPackage ? '\'$packageExpression\$_assetName\'' : '_assetName'},
boundsNode: boundsNode,
animation: animation,
fit: fit,
Expand All @@ -58,7 +58,7 @@ class FlareIntegration extends Integration {

String get path => _assetName;

String get keyName => ${packageExpression == null ? '_assetName' : '\'$packageExpression\$_assetName\''};
String get keyName => ${isPackage ? '\'$packageExpression\$_assetName\'' : '_assetName'};
}''';

@override
Expand Down
1 change: 1 addition & 0 deletions packages/core/lib/generators/integrations/integration.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ abstract class Integration {
Integration(this.packageParameterLiteral);

final String packageParameterLiteral;
late final bool isPackage = packageParameterLiteral.isNotEmpty;

bool isEnabled = false;

Expand Down
Loading