-
Notifications
You must be signed in to change notification settings - Fork 59
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
feat: add types to graphql in TS outputs #620
Changes from all commits
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 |
---|---|---|
|
@@ -7,12 +7,30 @@ const LINE_DELIMITOR = '\n'; | |
* Utility class to format the generated GraphQL statements based on frontend language type | ||
*/ | ||
class GraphQLStatementsFormatter { | ||
constructor(language) { | ||
constructor(language, op) { | ||
this.language = language || 'graphql'; | ||
this.opTypeName = { | ||
queries: 'Query', | ||
mutations: 'Mutation', | ||
subscriptions: 'Subscription', | ||
}[op]; | ||
this.lintOverrides = []; | ||
this.headerComments = []; | ||
} | ||
|
||
get typeDefs() { | ||
if (this.language === 'typescript' && this.opTypeName) { | ||
return [ | ||
`import * as APITypes from '../API';`, | ||
`type Generated${this.opTypeName}<InputType, OutputType> = string & {`, | ||
` __generated${this.opTypeName}Input: InputType;`, | ||
` __generated${this.opTypeName}Output: OutputType;`, | ||
`};`, | ||
].join(LINE_DELIMITOR); | ||
} | ||
return ''; | ||
} | ||
|
||
format(statements) { | ||
switch (this.language) { | ||
case 'javascript': | ||
|
@@ -21,10 +39,7 @@ class GraphQLStatementsFormatter { | |
return this.prettify(this.formatJS(statements)); | ||
case 'typescript': | ||
this.headerComments.push(CODEGEN_WARNING); | ||
this.lintOverrides.push(...[ | ||
'/* tslint:disable */', | ||
'/* eslint-disable */' | ||
]); | ||
this.lintOverrides.push(...['/* tslint:disable */', '/* eslint-disable */']); | ||
return this.prettify(this.formatJS(statements)); | ||
case 'flow': | ||
this.headerComments.push('@flow', CODEGEN_WARNING); | ||
|
@@ -36,27 +51,42 @@ class GraphQLStatementsFormatter { | |
} | ||
|
||
formatGraphQL(statements) { | ||
const headerBuffer = this.headerComments.map( comment => `# ${comment}`).join(LINE_DELIMITOR); | ||
const headerBuffer = this.headerComments.map(comment => `# ${comment}`).join(LINE_DELIMITOR); | ||
const statementsBuffer = statements ? [...statements.values()].join(LINE_DELIMITOR) : ''; | ||
const formattedOutput = [headerBuffer, LINE_DELIMITOR, statementsBuffer].join(LINE_DELIMITOR); | ||
return formattedOutput; | ||
} | ||
|
||
formatJS(statements) { | ||
const lintOverridesBuffer = this.lintOverrides.join(LINE_DELIMITOR); | ||
const headerBuffer = this.headerComments.map( comment => `// ${comment}`).join(LINE_DELIMITOR); | ||
const headerBuffer = this.headerComments.map(comment => `// ${comment}`).join(LINE_DELIMITOR); | ||
const formattedStatements = []; | ||
if (statements) { | ||
for (const [key, value] of statements) { | ||
formattedStatements.push( | ||
`export const ${key} = /* GraphQL */ \`${value}\`` | ||
); | ||
const typeTag = this.buildTypeTag(key); | ||
formattedStatements.push(`export const ${key} = /* GraphQL */ \`${value}\`${typeTag}`); | ||
} | ||
} | ||
const formattedOutput = [lintOverridesBuffer, headerBuffer, LINE_DELIMITOR, ...formattedStatements].join(LINE_DELIMITOR); | ||
const formattedOutput = [lintOverridesBuffer, headerBuffer, LINE_DELIMITOR, this.typeDefs, LINE_DELIMITOR, ...formattedStatements].join( | ||
LINE_DELIMITOR, | ||
); | ||
return formattedOutput; | ||
} | ||
|
||
buildTypeTag(name) { | ||
if (!this.opTypeName) return ''; | ||
if (this.language !== 'typescript') return ''; | ||
|
||
const titleCasedName = `${name[0].toUpperCase()}${name.slice(1)}`; | ||
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. If I'm reading things right, what I'm deriving here is actually 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. Added as another PR to compare/contrast: https://github.com/aws-amplify/amplify-codegen/pull/623/files |
||
const variablesTypeName = `APITypes.${titleCasedName}${this.opTypeName}Variables`; | ||
const resultTypeName = `APITypes.${titleCasedName}${this.opTypeName}`; | ||
Comment on lines
+81
to
+82
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'll have a rev by standup tomorrow that uses 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. Added as another PR to compare/contrast: https://github.com/aws-amplify/amplify-codegen/pull/623/files |
||
|
||
return ` as Generated${this.opTypeName}< | ||
${variablesTypeName}, | ||
${resultTypeName} | ||
>;`; | ||
} | ||
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 was initially thinking this was very gross. But, now I'm wondering if the |
||
|
||
prettify(output) { | ||
const parserMap = { | ||
javascript: 'babel', | ||
|
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.
Is the location of
API.ts
stable relative to the generated graphql path? I'm guessing not. How should I calculate the correct relative path here?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.
No it is not, customers can choose any custom path relative to the project root when they do "amplify add codegen".