Skip to content

[native_assets_cli] Try to minimize the amount of dependencies #1000

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

Open
dcharkes opened this issue Mar 12, 2024 · 2 comments
Open

[native_assets_cli] Try to minimize the amount of dependencies #1000

dcharkes opened this issue Mar 12, 2024 · 2 comments
Labels
P3 A lower priority bug or feature request package:hooks

Comments

@dcharkes
Copy link
Collaborator

dcharkes commented Mar 12, 2024

As many packages may depend on this one, I think it would be nice if we could limit the dependencies to an absolute minimum.

#946 (comment)

$ dart pub deps
native_assets_cli 0.5.0-wip
├── cli_config 0.2.0
│   ├── args 2.4.2
│   └── yaml...
├── collection 1.18.0
├── crypto 3.0.3
│   └── typed_data 1.3.2
│       └── collection...
├── logging 1.2.0
├── pub_semver 2.1.4
│   ├── meta 1.11.0
│   └── collection...
├── yaml 3.1.2
│   ├── source_span 1.10.0
│   │   ├── term_glyph 1.2.1
│   │   ├── collection...
│   │   └── path...
│   ├── string_scanner 1.2.0
│   │   └── source_span...
│   └── collection...
└── yaml_edit 2.1.1
    ├── collection...
    ├── meta...
    ├── source_span...
    └── yaml...

(manually deleted dev dependencies)

To keep:

  • collection: Needed for equality checks, hashcodes etc. Unlikely to ever become 2.0.
  • meta: Unlikely to ever become 2.0.
  • pub_semver: Unlikely to ever become 2.0.

To argue:

  • logging: Love it or hate it, but standardizing a logger in a layered architecture makes things so much easier. (E.g. flutter_tools calls native_assets_builder, calls native_assets_cli, calls cli_config, which parses YAML, and at every level we can log errors. Which means we don't have to catch a gazillion different exceptions everywhere in the code.) We could come up with our own logging abstraction, but we'd need to share it across a layer of packages.

To remove:

  • crypto: We only use it for hashing the BuildConfig fields. I'm not aware of such functionality in dart:. We could probably move the code to native_assets_builder though. Or we could take an hashing function argument.
  • yaml & yaml_edit: Eventually, we'll delete the dependency ([native_assets_builder] build input and output should be JSON instead of YAML #991).
    • We'll also need to remove cli_config as a dependency then.
@dcharkes dcharkes added P3 A lower priority bug or feature request package:hooks labels Mar 12, 2024
@dcharkes dcharkes mentioned this issue Jul 26, 2024
1 task
@mosuem
Copy link
Member

mosuem commented Jul 26, 2024

Comment on testing hooks: This might introduce more dependencies, such as package:test, but there is no real way around this. Extracting testing into a separate package doesn't help, as anyone using package:hook would also use package:hook_test.
To get the name of the current package, parsing the pubspec introduces a dependency on package:yaml, so we might not remove it after all.

@dcharkes
Copy link
Collaborator Author

Extracting testing into a separate package doesn't help, as anyone using package:hook would also use package:hook_test.

They would add the dependencies to dev_dependencies right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 A lower priority bug or feature request package:hooks
Projects
None yet
Development

No branches or pull requests

2 participants