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.

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

[swift] New 'swift-combine' client generator for swift #15823

Merged
merged 33 commits into from
Jun 22, 2023

Conversation

dydus0x14
Copy link
Contributor

@dydus0x14 dydus0x14 commented Jun 12, 2023

iOS team of my current project used existing Swift generator for a year, but I decided to implement and open source an alternative solution to support critical features for our project:

  • No 3rd-party dependencies used in generated code
  • No static methods and global scopes for APIs: it allows to reuse and customize APIs in one app
  • Multiply openapi files support: generator creates one OpenAPITransport library for all APIs as a dependency

Other nuances:

  • It is Apple Combine framework based solution
  • Applied optimizations for generated code:
    • less runtime types checks as possible
    • some utils methods defined in generated code only if used.
  • It is first basic version and does not support all features of OpenAPI schema. Still a lot of things to do.

Examples: https://github.com/dydus0x14/openapi-generator-swift-combine

Some metrics: currently my iOS project contains 19 open api files (20k of YAML and JSON lines) and 25k lines of code generated by swift-combine generator.

Swift technical committee: @jgavris @ehyche @Edubits @jaz-ah @4brunu

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • Run the following to build the project and update samples:
    ./mvnw clean package 
    ./bin/generate-samples.sh
    ./bin/utils/export_docs_generators.sh
    
    Commit all changed files.
    This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
    These must match the expectations made by your contribution.
    You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
    For Windows users, please run the script in Git BASH.
  • In case you are adding a new generator, run the following additional script :
    ./bin/utils/ensure-up-to-date
    
    Commit all changed files.
  • File the PR against the correct branch: master (6.3.0) (minor release - breaking changes with fallbacks), 7.0.x (breaking changes without fallbacks)
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@wing328
Copy link
Member

wing328 commented Jun 13, 2023

thanks for the PR. can this be added to swift5 via the library option instead? currently the following is supported:

urlsession
  [DEFAULT] HTTP client: URLSession
alamofire
  HTTP client: Alamofire
vapor
  HTTP client: Vapor

@4brunu
Copy link
Contributor

4brunu commented Jun 13, 2023

Hi, thanks for taking the time to improve openapi-generator.
Like @wing328 I would prefer to take those improvements into the existing generator, rather than creating a new one.
For example, No static methods and global scopes for APIs: it allows to reuse and customize APIs in one app is something that other people have asked in the past.

@dydus0x14
Copy link
Contributor Author

Hi @4brunu @wing328 and thanks for your feedback!

I thought about updating current generator instead of creating new one, but it seemed too difficult for me to support all described above bullet points for existing swift generator (due to architecture change and bunch of existing library options to support) and do not break backward compatibility.

How would you see a point about 'No static methods' might be integrated to the current generator? As I see there are several options:

  • Without the library options, just change current APIs signatures in templates and break backward compatibility.
  • With some library option which replace static with no static methods

@4brunu
Copy link
Contributor

4brunu commented Jun 14, 2023

Hi, I think we need to have an option to let people decide which one they want to use.

@dydus0x14
Copy link
Contributor Author

Ok, so if we do not interested in new generator I may close this PR and open new one for 'No static methods' feature for existing generator once it's ready.

@wing328
Copy link
Member

wing328 commented Jun 14, 2023

I'm ok to accept it to give people another option and to try it out so as to collect more feedback.

@4brunu and I suggested adding it to the existing swift5 generator since it's easier to maintain based on our experience. If it's too hard to add swift-combine into the existing swift5 generator via the library option, we won't go that path.

@wing328
Copy link
Member

wing328 commented Jun 14, 2023

can you please update bitrise.yml to run tests for swift-combine petstore samples?

@dydus0x14
Copy link
Contributor Author

@wing328 done

@wing328 wing328 closed this Jun 14, 2023
@wing328 wing328 reopened this Jun 14, 2023
@wing328
Copy link
Member

wing328 commented Jun 15, 2023

the birise CI tests failed: https://app.bitrise.io/build/10f86487-ec07-44be-9058-5a5f7fe42535

can you please take a look when you've time?

@dydus0x14
Copy link
Contributor Author

the birise CI tests failed: https://app.bitrise.io/build/10f86487-ec07-44be-9058-5a5f7fe42535

can you please take a look when you've time?

Sure, looks like I fixed it, but have no idea how to rerun bitrise pipelines.

@wing328
Copy link
Member

wing328 commented Jun 20, 2023

trigger a build in https://app.bitrise.io/build/5f640a77-25ad-42f1-bf05-e5d206ea804c. let's see how that goes

@wing328
Copy link
Member

wing328 commented Jun 20, 2023

@wing328
Copy link
Member

wing328 commented Jun 20, 2023

successfully triggered a build failure too so all good

@dydus0x14 can you please PM me via Slack when you've time? Thank you.

@wing328 wing328 added this to the 7.0.0 milestone Jun 20, 2023
@wing328
Copy link
Member

wing328 commented Jun 22, 2023

circieci failure not related to this change.

@@ -0,0 +1,817 @@
/*
* Copyright 2018 OpenAPI-Generator Contributors (https://openapi-generator.tech)
* Copyright 2018 SmartBear Software
Copy link
Member

Choose a reason for hiding this comment

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

@wing328 wing328 merged commit 9f3d9a5 into OpenAPITools:master Jun 22, 2023
15 of 16 checks passed
fmoraespadtec pushed a commit to padteclab/openapi-generator that referenced this pull request Jun 26, 2023
…15823)

* swift-alt-gen init

* swift-alt-gen in progress

* swift-alt added runtime

* swift-alt added transport

* swift-alt added example

* swift-alt v0.1.0

* swift-alt implemented form encoded body

* swift-alt fixed array of enums to string

* swift-alt v0.2.0

* swift-alt v0.3.0

* swift-alt v0.4.0

* swift-alt v0.5.0

* swift-alt v0.6.0

* swift-alt v0.7.0

* swift-alt v0.8.0

* swift-alt v0.9.0

* swift-alt v0.12.0

* swift-alt v0.13.0

* swift-alt v0.14.0

* swift-alt v0.15.0

* swift-alt v0.16.0

* swift-alt v0.17.0

* swift-alt v0.18.0

* swift-alt v0.19.0 Support for raw value in header

* swift-alt v0.20.0

* swift-alt v0.21.0

* swift-alt v0.22.0

* swift-combine v0.23.0

* swift-combine PR rules adoption

* swift-combine: updated transport

* Updated bitrise.yml file

* Fixed bitrise pipeline for swift-combine

* Fixed code review comment
@tivadarmolitorisz
Copy link

Hi @dydus0x14, why did you use Combine? It's basically dead, async/await should be used instead. Your basic idea is great, to have a solution without 3rd party dependencies, etc, so the question is, are you/your team planning to replace this approach?

@dydus0x14
Copy link
Contributor Author

Hi @tivadarmolitorisz, thanks! Actually I implemented and used this generator for my project much earlier than WWDC 2023, so I had no idea about such move from Apple side😁 But I still decided to open source it because someone may still use Combine framework for like service layer in app. Now I do not have any plans to get away from Combine in my project, but it might be a great idea to create the similar Swift generator with async/await public signatures. So if you or your team is interested to use it, I can do it.

@tivadarmolitorisz
Copy link

@dydus0x14 we're interested in it, probably you should look for a new name for your generator (not swift-combine) and support the async-await operations as well, for example, swift-nextgen.

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

Successfully merging this pull request may close these issues.

None yet

5 participants