Swift: Fix regression for individual files.

[designatednerd]

#1449 Pointed out that #1241 introduced a regression when files are being output individually. This PR updates our logic to make sure that information is shared and then fix the issue.

[gsabran]

Thanks for making this PR so promptly!
Do you also need to change isRedundant here

apollo-tooling/packages/apollo-codegen-swift/src/codeGeneration.ts

Line 175 in 2843a43

const isRedundant = !!namespace;

to const isRedundant = outputIndividualFiles && !!namespace;?

[designatednerd]

@gsabran Guh, good catch - there were a couple things that seem to have not made the bus on my last commit.

[designatednerd]

I'm gonna take another look at this in the morning before merging - @JacksonKearl let me know if any of the TS looks fishy to you

[gsabran]

Isn't the codegen written with extensions when outputIndividualFiles is true? In that case this should be

const isRedundant = !!namespace && outputIndividualFiles;

right?

[designatednerd]

You're correct that it's written with extensions when outputIndividualFiles is true, but isRedundant should only ever be true if we're not outputting individual files.

If isRedundant is true, codegen omits the public, which is the behavior filed as a bug.

[designatednerd]

After figuring out the thing I fucked up, yep, you're right!

[gsabran]

Same question regarding whether !outputIndividualFiles should be outputIndividualFiles

[designatednerd]

same answer

[designatednerd]

@gsabran I'll wait on your 👍to merge this one - let me know if you have more questions!

[gsabran]

public is redundant for extensions so the desired outputs are

public enum MyNameSpace {}

public extension MyNameSpace {
   struct Hero: GraphQLFragment { // <- public would be redundant here
        public var name: String
        public var age: Int
    }
}

but not for enums as

public enum MyNameSpace {
    public struct Hero: GraphQLFragment { // <- public is required here
        public var name: String
        public var age: Int
    }
}

so I think we should only look for redundancy when the codegen generates extensions, ie when it outputs multiple files. Maybe I'm misunderstanding what outputIndividualFiles stands for.

[designatednerd]

AHA. I think I see where I missed something - the JS !! operator works different than I thought it did. Good catch!

[designatednerd]

@gsabran better?

[JacksonKearl]

Seems reasonable on the TS side, only thing I might add is more documentation around what the parameter is doing, perhaps in the form of some docstrings. As is, I think it could be difficult for someone adding a new feature in the future who need to use one of these functions to know if they should be passing true or false for that parameter.

[JacksonKearl]

I'd add a docstring to describe what exactly this parameter is doing (maybe with examples of the difference in output?), because it's not very clear to me just looking at it.

[JacksonKearl]

same here re docstring

[designatednerd]

@gsabran @JacksonKearl Would love 👍s from you guys if you think this is ready to go - would love to get this merged soon so we can put it out with a patch release

[gsabran]

thanks for making this change!