-
Notifications
You must be signed in to change notification settings - Fork 470
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
[TS] Dedup enums and inputs by using global types file #520
Changes from all commits
9914a0e
eb86363
76b9462
2a9a24b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,7 @@ import { generateSource as generateSwiftSource } from "apollo-codegen-swift"; | |
import { generateSource as generateTypescriptLegacySource } from "apollo-codegen-typescript-legacy"; | ||
import { generateSource as generateFlowLegacySource } from "apollo-codegen-flow-legacy"; | ||
import { generateSource as generateFlowSource } from "apollo-codegen-flow"; | ||
import { generateSource as generateTypescriptSource } from "apollo-codegen-typescript"; | ||
import { generateLocalSource as generateTypescriptLocalSource, generateGlobalSource as generateTypescriptGlobalSource } from "apollo-codegen-typescript"; | ||
import { generateSource as generateScalaSource } from "apollo-codegen-scala"; | ||
import { GraphQLSchema } from "graphql"; | ||
import { FlowCompilerOptions } from '../../apollo-codegen-flow/lib/language'; | ||
|
@@ -71,12 +71,9 @@ export default function generate( | |
writeOperationIdsMap(context); | ||
writtenFiles += 1; | ||
} | ||
} else if (target === "flow" || target === "typescript" || target === "ts") { | ||
} else if (target === "flow") { | ||
const context = compileToIR(schema, document, options); | ||
const { generatedFiles, common } = | ||
target === "flow" | ||
? generateFlowSource(context) | ||
: generateTypescriptSource(context); | ||
const { generatedFiles, common } = generateFlowSource(context); | ||
|
||
const outFiles: { | ||
[fileName: string]: BasicGeneratedFile; | ||
|
@@ -118,6 +115,52 @@ export default function generate( | |
generatedFiles.map(o => o.content.fileContents).join("\n") + common | ||
); | ||
|
||
writtenFiles += 1; | ||
} | ||
} else if (target === "typescript" || target === "ts") { | ||
const context = compileToIR(schema, document, options); | ||
const generatedFiles = generateTypescriptLocalSource(context); | ||
const generatedGlobalFile = generateTypescriptGlobalSource(context); | ||
|
||
const outFiles: { | ||
[fileName: string]: BasicGeneratedFile; | ||
} = {}; | ||
|
||
if (nextToSources || (fs.existsSync(outputPath) && fs.statSync(outputPath).isDirectory())) { | ||
if (nextToSources && !fs.existsSync(outputPath)) { | ||
fs.mkdirSync(outputPath); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this case you want to be checking and creating the joined path, right? I think you might need to move the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure I understand what you are trying to say. This line would create the directory for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I was confusing |
||
} | ||
|
||
const globalSourcePath = path.join(outputPath, "globalTypes.ts"); | ||
outFiles[globalSourcePath] = { | ||
output: generatedGlobalFile.fileContents, | ||
}; | ||
|
||
generatedFiles.forEach(({ sourcePath, fileName, content }) => { | ||
let dir = outputPath; | ||
if (nextToSources) { | ||
dir = path.join(path.dirname(sourcePath), dir); | ||
if (!fs.existsSync(dir)) { | ||
fs.mkdirSync(dir); | ||
} | ||
} | ||
|
||
const outFilePath = path.join(dir, fileName); | ||
outFiles[outFilePath] = { | ||
output: content({ outputPath: outFilePath, globalSourcePath }).fileContents, | ||
}; | ||
}); | ||
|
||
writeGeneratedFiles(outFiles, path.resolve(".")); | ||
|
||
writtenFiles += Object.keys(outFiles).length; | ||
} else { | ||
fs.writeFileSync( | ||
outputPath, | ||
generatedFiles.map(o => o.content().fileContents).join("\n") + | ||
generatedGlobalFile.fileContents, | ||
); | ||
|
||
writtenFiles += 1; | ||
} | ||
} else { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic still seems pretty similar to Flow other than the change in the call to
content
. Would it be possible to extract out the common logic?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, you could probably use the TypeScript code for Flow as is. The Flow generator would receive the additional property
globalSourcePath
but it would just ignore it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the separation was with intent. Yes there is a lot of logic, but there is also apollographql/rover#410 which I didn't want to interfere with. I wanted to keep them separate until Flow catches up and dedups as well. Then those paths could be merged again (if similar enough). What do you think?
If we use the TS code as is for flow, we would also have to modify the flow package to return content as a closure. (Which I am not really interested in doing to be honest as I don't care much about Flow.)