Skip to content

Conversation

@FionaBronwen
Copy link

@FionaBronwen FionaBronwen commented Jan 5, 2026

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 transformations
  • test/lib/type-utils.test.ts - Unit tests for sanitizeNameForGraphQL

Utilities Added

Function Purpose
sanitizeNameForGraphQL Sanitize names to be valid GraphQL identifiers
toTypeName Convert to PascalCase for type names
toFieldName Convert to camelCase for field names
toEnumMemberName Convert to CONSTANT_CASE for enum members
getUnionName Generate names for anonymous unions
getTemplatedModelName Generate names for templated models (e.g., ListOfString)
isArray, isRecordType Type guards for array/record models
unwrapModel, unwrapType Extract element types from arrays
isTrueModel Check if a model should emit as GraphQL object type
getGraphQLDoc Extract doc comments for GraphQL descriptions

@FionaBronwen FionaBronwen force-pushed the fionabronwen/type-utils branch 3 times, most recently from 2606331 to 65f55e7 Compare January 7, 2026 19:55
@FionaBronwen FionaBronwen force-pushed the fionabronwen/type-utils branch from 65f55e7 to 0bc74f1 Compare January 7, 2026 19:59
@FionaBronwen FionaBronwen marked this pull request as ready for review January 7, 2026 20:03
@FionaBronwen FionaBronwen removed the request for review from swatkatz January 7, 2026 20:03
Comment on lines +236 to +246
if (!program.compilerOptions.miscOptions?.isTest) {
doc =
(doc || "") +
`

Created from ${type.kind}
\`\`\`
${getTypeName(type)}
\`\`\`
`;
}

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));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
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: "" },

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.

Comment on lines +178 to +190
/**
* 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");
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/**
* 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants