Skip to content

swift: Fix codegen with empty input objects / arrays#1589

Merged
jbaxleyiii merged 1 commit intoapollographql:masterfrom
lilyball:swift_empty_field_arguments
Oct 17, 2019
Merged

swift: Fix codegen with empty input objects / arrays#1589
jbaxleyiii merged 1 commit intoapollographql:masterfrom
lilyball:swift_empty_field_arguments

Conversation

@lilyball
Copy link
Copy Markdown
Contributor

@lilyball lilyball commented Oct 17, 2019

The codegen was omitting the [:] and [] tokens entirely.

Fixes apollographql/apollo-ios#838

The root cause of missing [:] was there was some code relying on the truthy value of strings, but the conversion to SwiftSource caused even empty sources to become truthy. I'm actually surprised TypeScript happily accepts someObject || someStr and evaluates it as the object type with no warnings (I mean, it will always be the object type, but the || someStr is now dead code).

Empty arrays I'm pretty sure were always broken even before SwiftSource due to the counterintuitive behavior of wrap (namely, wrap("[", "", "]") == "" instead of returning "[]"). I didn't want to deviate too much from the underlying wrap behavior (since the behavior is relied-upon in places) so I just made it return undefined instead of an empty source object, which forced us to handle that at the call sites.

TODO:

  • Update CHANGELOG.md* with your change (include reference to issue & this PR)
  • Make sure all of the significant new logic is covered by tests
  • Rebase your changes on master so that they can be merged easily
  • Make sure all tests and linter rules pass

*Make sure changelog entries note which project(s) has been affected. See older entries for examples on what this looks like.

@lilyball lilyball force-pushed the swift_empty_field_arguments branch from 1ba3610 to 5bab071 Compare October 17, 2019 00:39
The codegen was omitting the `[:]` and `[]` tokens entirely.

Fixes apollographql/apollo-ios#838
@lilyball lilyball force-pushed the swift_empty_field_arguments branch from 5bab071 to 3f3d260 Compare October 17, 2019 02:28
@designatednerd
Copy link
Copy Markdown
Contributor

@abernix Any idea what's going on with the failure on Node 8? Seems unrelated.

@lilyball Thanks for this - @cltnschlosser When you have a chance can you confirm this fixes your issue please? I believe you should be able to pull the updated CLI zip from the artifacts tab of the Build CLI circle job (grab the one that says Darwin, that includes node).

@designatednerd designatednerd added 🐦 component - swift 🤖 component - codegen related to the codegen core packages labels Oct 17, 2019
@cltnschlosser
Copy link
Copy Markdown
Contributor

@designatednerd Couldn't find the correct artifact, but I pulled @lilyball 's branch and confirmed that it fixes the issue. Thanks for the quick work!

@designatednerd
Copy link
Copy Markdown
Contributor

Great - kicked the build again, @abernix wasn't really sure what the hell was going on

@jbaxleyiii jbaxleyiii merged commit 35735dd into apollographql:master Oct 17, 2019
@lilyball lilyball deleted the swift_empty_field_arguments branch October 17, 2019 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐦 component - swift 🤖 component - codegen related to the codegen core packages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compile error of generated code with 0.17.0

4 participants