-
Notifications
You must be signed in to change notification settings - Fork 1
Add GraphQL name sanitization utilities #55
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
base: feature/graphql
Are you sure you want to change the base?
Conversation
2606331 to
65f55e7
Compare
65f55e7 to
0bc74f1
Compare
| if (!program.compilerOptions.miscOptions?.isTest) { | ||
| doc = | ||
| (doc || "") + | ||
| ` | ||
|
|
||
| Created from ${type.kind} | ||
| \`\`\` | ||
| ${getTypeName(type)} | ||
| \`\`\` | ||
| `; | ||
| } |
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.
We should leave this out; this was for internal debugging only.
In https://github.com/pinternal/pinboard/commit/2b03704068544b570ab1cb5cb065ecfc74c05134 it was changed to be behind an emitter config option, which I don't think we need to support here.
| options: { conjunction: string; prefix: string } = { conjunction: "And", prefix: "" }, | ||
| ): string { | ||
| if (isTemplateInstance(type)) { | ||
| const args = type.templateMapper.args.filter(isNamedType).map((arg) => getTypeName(arg)); |
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.
nit:
| const args = type.templateMapper.args.filter(isNamedType).map((arg) => getTypeName(arg)); | |
| const args = type.templateMapper.args.filter(isNamedType).map(getTypeName); |
|
|
||
| function getTemplateStringInternal( | ||
| args: string[], | ||
| options: { conjunction: string; prefix: string } = { conjunction: "And", prefix: "" }, |
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.
All of the prefixing logic (here and elsewhere) should be removed from these utils. In a mutator world, it makes a lot more sense as a separate (and very straightforward) mutation.
| /** | ||
| * Check if a model is an array type. | ||
| */ | ||
| export function isArray(model: Model): model is ArrayModelType { | ||
| return Boolean(model.indexer && model.indexer.key.name === "integer"); | ||
| } | ||
|
|
||
| /** | ||
| * Check if a model is a record/map type. | ||
| */ | ||
| export function isRecordType(type: Model): type is RecordModelType { | ||
| return Boolean(type.indexer && type.indexer.key.name === "string"); | ||
| } |
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.
| /** | |
| * Check if a model is an array type. | |
| */ | |
| export function isArray(model: Model): model is ArrayModelType { | |
| return Boolean(model.indexer && model.indexer.key.name === "integer"); | |
| } | |
| /** | |
| * Check if a model is a record/map type. | |
| */ | |
| export function isRecordType(type: Model): type is RecordModelType { | |
| return Boolean(type.indexer && type.indexer.key.name === "string"); | |
| } |
As noted here, these should be removed in favor of using the equivalent functions in typespec/compiler. Those take a program argument but don't actually use it, so perhaps we can open a PR to deprecate the argument and make it optional.
Summary
Adds utility functions for transforming TypeSpec names into valid GraphQL identifiers. These utilities form the foundation for name handling throughout the GraphQL emitter.
Changes
src/lib/type-utils.ts- Core utility functions for GraphQL name transformationstest/lib/type-utils.test.ts- Unit tests forsanitizeNameForGraphQLUtilities Added
sanitizeNameForGraphQLtoTypeNametoFieldNametoEnumMemberNamegetUnionNamegetTemplatedModelNameListOfString)isArray,isRecordTypeunwrapModel,unwrapTypeisTrueModelgetGraphQLDoc