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

feature: add support for object input default values and add support for default inputs to kotlin2 #680

Merged
merged 5 commits into from
Jun 24, 2024

Conversation

dwilkolek
Copy link
Contributor

@dwilkolek dwilkolek commented Apr 21, 2024

  • Added support for object default input param to kotlin and java input types
  • Implemented default input params for kotlin2 generator

closes #297

@dwilkolek dwilkolek force-pushed the fix/297 branch 2 times, most recently from c521a26 to 523569e Compare April 21, 2024 14:19
@dwilkolek dwilkolek marked this pull request as ready for review April 21, 2024 14:24
Copy link
Contributor

@srinivasankavitha srinivasankavitha left a comment

Choose a reason for hiding this comment

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

The changes look good to me. Would you also be able to fix the Java code path as well as part of this PR? If not, we can open a new issue for it.

@dwilkolek
Copy link
Contributor Author

Sure, I'll take a stab at it shortly.

@dwilkolek
Copy link
Contributor Author

dwilkolek commented Apr 22, 2024

While peeking at java code path I've found a bug in kotlin path. When using typeMapping with Person to DifferentlyNamedClass mapping then it would fail.

Regarding Java path it will be slightly more difficult. In Kotlin path I used named parameters. In Java I see 4 paths:

  1. create json object and use ObjectMapper to create object in place
  2. use default no param constructor and call setters in initialiser block
  3. use constructor with arguments but it will require the same order and matching constructor for mapped types
  4. use builder - this would be great for generated classes, but will most likely fail for type mapped classes.

@dwilkolek
Copy link
Contributor Author

I've implemented option 2. Lmk what do you think 🙏

@dwilkolek
Copy link
Contributor Author

I realised that kotlin2 did not support default values at all. Now experience with default values should be the same across codegen configurations.

@dwilkolek dwilkolek changed the title fix: add support for object default values to kotlin codegen feature: add support for object input default values and add support for default inputs to kotlin2 May 2, 2024
@dwilkolek dwilkolek force-pushed the fix/297 branch 2 times, most recently from 9288bff to 9aa4b1f Compare May 14, 2024 16:59
@dwilkolek
Copy link
Contributor Author

dwilkolek commented May 14, 2024

@mbossenbroek This PR contains kotlin2 changes - added support for default values in input types 😉
@srinivasankavitha I've implemented java code path using initialiser block

If it make sense to you I can split changes into 2 PRs; one for object defaults, one adding kotlin2 default values

@mbossenbroek
Copy link
Contributor

mbossenbroek commented May 14, 2024

Looks good to me! 👍 I'll let @srinivasankavitha merge it cause there are other changes in there too

@dwilkolek
Copy link
Contributor Author

@srinivasankavitha Are you waiting for any action from my side or is it ready to go? I'm not a huge fan of stale PRs hanging out in void 😅

@srinivasankavitha srinivasankavitha merged commit d11a021 into Netflix:master Jun 24, 2024
2 checks passed
@srinivasankavitha
Copy link
Contributor

Sorry for the delay, and thanks for bringing this back to my attention. Just merged the PR and will go out in next week's release.

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

Successfully merging this pull request may close these issues.

bug: Kotlin GraphQL Code Gen on input type with default value is not correct
3 participants