OpenAPITools / openapi-generator

OpenAPI Generator allows generation of API client libraries (SDK generation), server stubs, documentation and configuration automatically given an OpenAPI Spec (v2, v3)
https://openapi-generator.tech
Apache License 2.0
21.76k stars 6.57k forks source link

[REQ] [dart-dio] Automatically call build_runner build #15448

Open ahmednfwela opened 1 year ago

ahmednfwela commented 1 year ago

Since dart-dio generates code the relies on build_runner I think it would be a good idea to call

dart pub get

and

dart pub run build_runner build

automatically after each generation

@jaumard (2018/09) @josh-burton (2019/12) @amondnet (2019/12) @sbu-WBT (2020/12) @kuhnroyal (2020/12) @agilob (2020/12) @ahmednfwela (2021/08)

kuhnroyal commented 1 year ago

We can probably do this in the post process handling and at the same team clean up the formatting which still defaults to the old dart formatter.

kuhnroyal commented 1 year ago

Now that the tests are run on Github we can also ensure consistent formatting for the sample projects.

kuhnroyal commented 1 year ago

There is no general infrastructure (ENV variable, CLI switch) for something like this. We have the DART_POST_PROCESS_FILE and --enable-post-process-file in line with other generators, documented here: docs/file-post-processing.md

This does not however allow us to do any processing over all files. And it is much slower to format singles files compared to formatting the whole project at once. So I would like to see a different approach but not sure how to proceed here.


Now that the tests are run on Github we can also ensure consistent formatting for the sample projects.

I remember talking about this with @wing328 like 2 years back and we decided against it, because that would break builds when there are changes in sample specs etc. for developers who don't have a Dart stack installed.

wing328 commented 1 year ago

for developers who don't have a Dart stack installed.

Right, we found that if there are more steps to just a submit a PR (for Dart in this case), it's unlikely the contributors will thoroughly follow all the steps (not blaming them, we still appreciate their contribution) and as a result, there are more over head for the TC to maintain the project (which is not something I want to see as I want o minimize overhead if possible)

ahmednfwela commented 1 year ago

@wing328 can't we add a flag to run the extra commands if opted in ?

wing328 commented 1 year ago

can't we add a flag to run the extra commands if opted in ?

If that's the case, what about running the extra commands post code generation instead of bundling that into openapi-generator-cli for example?

ahmednfwela commented 1 year ago

you mean like DART_POST_PROCESS_FILE ? @wing328

wing328 commented 1 year ago

nope

I mean like just run openapi-generator-cli to generate something and then run the code formatter/linter in the next command.

(I think that's what most of the users are doing if they want to format the output)

ahmednfwela commented 1 year ago

yes, but the problem is the code won't work without running dart run build_runner build, it would show a compile time error this is why we have POM files here inside dart samples https://github.com/OpenAPITools/openapi-generator/blob/master/samples/openapi3/client/petstore/dart-dio/petstore_client_lib_fake/pom.xml#L43-L55

wing328 commented 1 year ago

the code won't work without running dart run build_runner build

No expert in dart. can you please elaborate on what this command do exactly?

For other Dart API client or SDKs (e.g. https://github.com/isoos/elastic_client) , do you they need to run the same command to make it work?

ahmednfwela commented 1 year ago

@wing328 since dart doesn't have reflection like c#, we can't know what a class looks like in runtime, so we use scripts that generate the code that needs reflection before we build the application (using dart build for example).

The official code generator by google is build_runner https://pub.dev/packages/build_runner and other packages depend on it like https://pub.dev/packages/built_value_generator and https://pub.dev/packages/json_serializable

but for each time we need to generate the reflection code, we have to call dart run build_runner build

For other Dart API client or SDKs (e.g. https://github.com/isoos/elastic_client) , do you they need to run the same command to make it work?

yes, every dart-dio API client has to run this command everytime code is generated by openapi-generator.

(but elastic_client) doesn't use code generator I think, as you can see in their pubspec.yaml, they don't have build_runner as dev_dependency https://github.com/isoos/elastic_client/blob/master/pubspec.yaml

wing328 commented 1 year ago

Thanks for the explanation. I've not checked but my guess is that you guys have mentioned running dart run build_runner build in the README as part of the client installation procedure, right?

My understanding is that other API cllients (manually written) also need to clearly inform the user to run dart run build_runner build manually as part of the installation process, right?

ahmednfwela commented 1 year ago

the short answer is, if a dart package has a build_runner dev_dependency, they need to run dart run build_runner build every time their code changes. but once you run it, other users don't need to run it again, since they can't change the source code of the package.

wing328 commented 1 year ago

I see. Let me try to come up with a solution. I think we both agree we would like to deliver a better experience to our Dart users using the generators.

kuhnroyal commented 1 year ago

We run this automatically as part of the CI for the samples.

But I also think we could use something like a POST_PROCESS switch which runs over the whole project and not only over single files. 7.0.0 could be a good time to introduce something like this.

wing328 commented 1 year ago

What about running dart run build_runner in public void postProcess() if the dart command if found.

If dart is not yet installed, show a warning instead and ask the users to run it manually later.

kuhnroyal commented 1 year ago

Yea I was thinking about using postProcess, I have a draft of that somewhere laying around.