Skip to content

[native_assets_cli] Should the API be not a main function? #152

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

Closed
dcharkes opened this issue Oct 5, 2023 · 2 comments
Closed

[native_assets_cli] Should the API be not a main function? #152

dcharkes opened this issue Oct 5, 2023 · 2 comments

Comments

@dcharkes
Copy link
Collaborator

dcharkes commented Oct 5, 2023

Should we consider using implementing an interface instead of having a main function for build.dart?

This would be in the style of the Builders of package:build.

Pros:

  • Visibility of conceptually different invocations of main:
    • dry-run vs build
    • before kernel compilation and after (edit: or build vs tree-shake)

Cons:

@dcharkes
Copy link
Collaborator Author

Notes from discussion with @mkustermann:

Cons:

  • Tight coupling between Dart/Flutter SDK and build.dart API. We would need to pin package:native_assets_cli like Flutter pins packages. A CLI based on YAML/JSON files with optional fields allows more slack.

@mkustermann Is this actually true con? If the API is instantiated by a JSON under the hood. And we generate a main.dart that basically parses the CLI and calls the methods from the API. Then adding optional fields would work in the same way as it works now. The situation we would have more tight coupling as you suggest is when we would actually import the code directly instead of going through a Process.run. But we can't actually do that, because dartdev could be a precompiled aot snapshot. So maybe your concern is invalid.

That being said, almost all build systems that we are aware of use scripts with a main: Rust build.rs, NodeJS post-install hooks, Python's setup.py. So it's probably best to stick with the convention of having a standalone script.

HosseinYousefi pushed a commit that referenced this issue Nov 16, 2023
Dependabot
GitHub Action
HosseinYousefi pushed a commit that referenced this issue Nov 16, 2023
Dependabot
GitHub Action
@dcharkes
Copy link
Collaborator Author

I don't think we're still considering this. We'll be sticking with a JSON protocol.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant