[native_assets_cli] Reduce number of visible libraries in package

[dcharkes]

https://pub.dev/documentation/native_assets_cli/latest/ has many libraries, which is quite confusing for users, especially because they re-export the same things over and over again.

All libraries that are internal should not generate docs. @mkustermann Any reason why the pkgs/native_assets_cli/lib/native_assets_cli_internal.dart ones show up in the docs again? The one internal was hidden before your refactoring with /// @nodoc. See https://pub.dev/documentation/native_assets_cli/0.8.0/. (Also, why did the rest of the doc comment disappear?)

Conceivably we need these to be public:

• pkgs/native_assets_cli/lib/native_assets_cli.dart (or maybe not, maybe these types should all be only re-exported in the individual asset exports?)
• pkgs/native_assets_cli/lib/code_assets.dart
• pkgs/native_assets_cli/lib/code_assets_testing.dart
• pkgs/native_assets_cli/lib/data_assets.dart

@mkustermann would like to really separate all the asset types and have separate imports for the separate asset types. However the downside is that now all classes show up in multiple libraries. We should consider:

1. Not re-exporting classes in multiple files. Users would have to import pkgs/native_assets_cli/lib/code_assets.dart and pkgs/native_assets_cli/lib/native_assets_cli.dart to write a hook.
2. Simply having a single import pkgs/native_assets_cli/lib/native_assets_cli.dart, a single library for users. (And maybe a second one for testing.)

Maybe @devoncarew and @mosuem Can give some guidance on best practises when trying to expose parts of packages via multiple libraries.

Another thing that is annoying with having separate imports and having extension types is that the IDE doesn't autocomplete the methods added in extension types in other imports, which hurts discoverability. This pushes towards option 2. (But doesn't fix discoverability issues with discoverability for extensions added in other packages. So it makes DataAssets and CodeAssets more privileged than other asset types.

[mkustermann]

All libraries that are internal should not generate docs. @mkustermann Any reason why the pkgs/native_assets_cli/lib/native_assets_cli_internal.dart ones show up in the docs again? The one internal was hidden before your refactoring with /// @Nodoc. See https://pub.dev/documentation/native_assets_cli/0.8.0/. (Also, why did the rest of the doc comment disappear?)

There should be no library that's "public" and at the same time "internal". It doesn't make any sense. What used to be native_assets_cli_internal.dart was not an internal library, other packages imported that library, so by definition it's part of the public API surface. Yes, the "excuse" for why one could've considered it to not be part of the public API surface is that this library is used by Dart SDK and Flutter SDK internals and both of them happen to pin the library (so no semver issues if breaking changes are made to it) - but this is a bad excuse, as any 3rd party may want to use the same functionality and they may not pin the package. So again, it's part of public API surface, we should treat it that way.

The split of the old library structure into code_assets.dart, data_assets.dart is to show the decoupling of those concepts. We may even want to move those libraries into their own packages (e.g. package:code_assets).

The separation between <xxx>.dart and <xxx>_builder.dart was made (partly based on review feedback I got ) that hook writers should get only what they need when they import their support library (e.g. in future package:code_assets/code_assets.dart). For the very rare case of someone writing a bundling tool (like Dart SDK & Flutter SDK) they would use the <xxx>_builder.dart library which contains things hook authors don't need things in there.

The code_assets_testing.dart is somewhat similar to before my refactorings where there was a test.dart IIRC. If that's really needed is questionable.

The reason for re-exports is that we should make things as easy for hook writer as we can, we should not force them to import package:native_assets_cli/native_assets_cli.dart (to get the build method and BuildConfig class) and then also package:code_asset/code_asset.dart to get code asset support. Making the ladder re-expose the former makes a hook writer only need to import one library instead of several - which is easier and I cannot find good reasons not to do that.

So in short: This library structure is showing the layering of the system: a base layer, code & data assets layered on top and cross-cutting concerns (bundling tool functionality, test functionality).

(We can probably get rid of the native_assets_cli_internal - this Hook class isn't really needed. Side note: I'd probably prefer the config.json saying where the output should be written, instead of the hook "knowing" to write this to a special "..._output.json" file)

Regarding the API doc generator: There's many improvements that could be made to it (e.g. for every class's api docs shows all inherited members e.g. get runtimeType - it's polluting the api docs with unnecessary information), similarly re-exported things could be displayed differently than repeating them.

[devoncarew]

Given the size of this package the large number of public libraries is a little confusing. I'd suggest either:

• have a single public library and have it re-export targeted API from lib/src; this is a pretty common pattern
• have a handful of public libraries - each with a good reason to exist and be separate. have a common public library most users will import; have more specific public libraries some users will import; don't have those specific public libraries re-export other public libraries (they'd export targeted API from lib/src)

[dcharkes]

For the very rare case of someone writing a bundling tool (like Dart SDK & Flutter SDK) they would use the <xxx>_builder.dart library which contains things hook authors don't need things in there.

Also hook helper packages such as package:native_toolchain_c use the _builder ones. The construct configs pass in to the builders.

We have three sets of users of this package.

• ~1000 (?) hook writers - needs data, code, and "base"
• ~10 (?) hook helper package writers (package:native_toolchain_rust etc.) - needs the _builders
• <10 embedder writers - needs the _internal

And then the first two sets of users are split again in:

• hook writers using producing code assets / hook helper package writers for code assets
• hook writers using producing data assets / hook helper package writers for data assets
• hook writers using producing other assets / hook helper package writers for other assets (so they need the base stuff but not the code or data)

That already creates 7 libraries.

And then we also split the _test ones off, of which we only have one for code right now: which makes 8.

I'd really like to bring this number down.

The code_assets_testing.dart is somewhat similar to before my refactorings where there was a test.dart IIRC. If that's really needed is questionable.

Yes, let's remove it.

I wonder if we can do anything to bring the 7 down further. Or if we can do anything to clarify for users what they should be importing. Maybe it should be spelled out in the library doc comments:

/// xxx
///
/// If you are a xxx, you should be importing xxx instead
library;

I'd still leaning towards hiding the APIs for the very rare use case of writing an embedder. I don't like regressing the experience for ~1000 users to cater for <10.

The split of the old library structure into code_assets.dart, data_assets.dart is to show the decoupling of those concepts. We may even want to move those libraries into their own packages (e.g. package:code_assets).

I agree that the concepts are decoupled.

I don't agree it's good for users to have them as separate imports. Theoretically speaking package test could ship without support for common matchers, it's up to the users to decide what matchers to import. Different types of matchers are different concepts. But in practice users are happy to have all the matchers imported. Batteries included.

[mkustermann]

I don't agree it's good for users to have them as separate imports. Theoretically speaking package test could ship without support for common matchers, it's up to the users to decide what matchers to import. Different types of matchers are different concepts. But in practice users are happy to have all the matchers imported. Batteries included.

Having them as separate packages will also serve another use: If we use the package's SDK constraint as a way to version the protocol extensions. So by having package:code_asset and package:data_asset those two protocol extensions can be versioned independently and soft-breaking changes can be made to one without breaking users of the other.

[dcharkes]

I don't agree it's good for users to have them as separate imports. Theoretically speaking package test could ship without support for common matchers, it's up to the users to decide what matchers to import. Different types of matchers are different concepts. But in practice users are happy to have all the matchers imported. Batteries included.

Having them as separate packages will also serve another use: If we use the package's SDK constraint as a way to version the protocol extensions. So by having package:code_asset and package:data_asset those two protocol extensions can be versioned independently and soft-breaking changes can be made to one without breaking users of the other.

Can you elaborate on this idea? We explored using SDK constraints before and ran into issues due to the SDK constraint of helper packages being able to be lower than the root package (#93). Doing it per asset type (per protocol extension) will not change that? Maybe just reopen that issue for elaborating on the idea. (Or are you talking about doing breaking changes to the Dart API of protocol extensions?)