Refactor file generators to be more functional #2132
Conversation
| E623FD2A2797A6F4008B4CED /* InterfaceTemplate.swift in Sources */ = {isa = PBXBuildFile; fileRef = E623FD292797A6F4008B4CED /* InterfaceTemplate.swift */; }; | ||
| E64F7EB827A0854E0059C021 /* UnionTemplate.swift in Sources */ = {isa = PBXBuildFile; fileRef = E64F7EB727A0854E0059C021 /* UnionTemplate.swift */; }; | ||
| E64F7EBA27A085D90059C021 /* UnionTemplateTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E64F7EB927A085D90059C021 /* UnionTemplateTests.swift */; }; | ||
| E64F7EBC27A11A510059C021 /* GraphQLNamedType+Swift.swift in Sources */ = {isa = PBXBuildFile; fileRef = E64F7EBB27A11A510059C021 /* GraphQLNamedType+Swift.swift */; }; | ||
| E64F7EBF27A11B110059C021 /* GraphQLNamedType+SwiftTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E64F7EBE27A11B110059C021 /* GraphQLNamedType+SwiftTests.swift */; }; | ||
| E64F7EC127A122300059C021 /* TypeTemplate.swift in Sources */ = {isa = PBXBuildFile; fileRef = E64F7EC027A122300059C021 /* TypeTemplate.swift */; }; | ||
| E64F7EC127A122300059C021 /* ObjectTemplate.swift in Sources */ = {isa = PBXBuildFile; fileRef = E64F7EC027A122300059C021 /* ObjectTemplate.swift */; }; |
There was a problem hiding this comment.
TypeTemplate and TypeFileGenerator have been renamed to ObjectTemplate and ObjectFileGenerator (and their relevant test files) to be consistent with naming concepts used elsewhere.
| @@ -31,6 +31,21 @@ public class CompilationResult: JavaScriptObject { | |||
| override public var debugDescription: String { | |||
| "\(name) on \(rootType.debugDescription)" | |||
| } | |||
|
|
|||
| lazy var nameWithSuffix: String = { | |||
There was a problem hiding this comment.
Moved out of OperationDefinitionTemplate so the file generator can use the logic too. Also lazy because it'll be called more than once.
| @@ -2,7 +2,7 @@ struct FragmentTemplate { | |||
|
|
|||
| let fragment: IR.NamedFragment | |||
| let schema: IR.Schema | |||
| let config: ApolloCodegenConfiguration | |||
| let config: ApolloCodegenConfiguration.FileOutput | |||
There was a problem hiding this comment.
These changes are narrowing the surface area of the configuration struct that gets passed around.
| } | ||
|
|
||
| // MARK: File Generator Tests | ||
|
|
||
| func test_fileGenerators_givenSchema_shouldCreateFileGeneratorsForUsedSchemaTypes() throws { | ||
| func test_fileGenerators_givenSchemaAndMultipleOperationDocuments_shouldGenerateSchemaAndOperationsFiles() throws { |
There was a problem hiding this comment.
This test is about verifying the complete file list output and not about the file paths specifically, therefore I don't intend to write additional tests with different configurations. My next PR that address the correct subfolder structure for file output will test the varying combinations and ensure that the correct file paths are generated.
| compilationResult: CompilationResult, | ||
| ir: IR, | ||
| config: ApolloCodegenConfiguration, | ||
| fileManager: FileManager = FileManager.default |
There was a problem hiding this comment.
I'm not a huge fan of having to expose fileManager in many more function definitions but I don't see another way around it for mocked testing; the alternative is actually writing to the file system.
At least this function is internal so users calling ApolloCodegen won't ever know this exists.
There was a problem hiding this comment.
Yeah, it's annoying but necessary. We should not be writing to the file system in unit tests.
| compilationResult: CompilationResult, | ||
| ir: IR, | ||
| config: ApolloCodegenConfiguration, | ||
| fileManager: FileManager = FileManager.default |
There was a problem hiding this comment.
Yeah, it's annoying but necessary. We should not be writing to the file system in unit tests.
| } | ||
|
|
||
| for operation in compilationResult.operations { | ||
| try autoreleasepool { |
There was a problem hiding this comment.
Yay for autoreleasepool!
| } | ||
|
|
||
| /// Designated initializer. | ||
| struct EnumFileGenerator { |
There was a problem hiding this comment.
Nit: We don't need to change this right now, but I usually use caseless enums for objects that are never initialized (only have static functions). That way it's literally impossible to even make an instance of them.
| let fileManager = MockFileManager(strict: false) | ||
|
|
||
| var filePaths: Set<String> = [] | ||
| fileManager.mock(closure: .createFile({ path, data, attributes in |
There was a problem hiding this comment.
Love this MockFileManager implementation!
This change is to have the file generators hold onto less data as the swift output files are being generated, which is particularly relevant for operation and fragment file generators which build large parts of the IR in their generation process.