Add TCGC exact-name support to TypeScript emitter - #5331
Add TCGC exact-name support to TypeScript emitter#5331Qiaoqiao Zhang (qiaozha) wants to merge 1 commit into
Conversation
microsoft-github-policy-service
Bot
added
the
emitter:typescript
|
All changed packages have been documented.
Show changes
|
📦 Package size report1 package changed size, +4.52 KB (+0.0%) packed overall.
12 package(s) with no notable change
Packed = gzipped |
commit: |
|
You can try these changes here
|
| if (!isValidTypeScriptIdentifierName(name)) { | ||
| return false; | ||
| } | ||
| const result = ts.transpileModule(`export const ${name} = 0;`, { |
There was a problem hiding this comment.
Would this transpileModule more time-consuming than a normal regex? Since the normalizeModelName function that calls this function is called from many places, do we need to add a cache?
| } | ||
|
|
||
| export function normalizeSdkPropertyName(sdkName: SdkName): string { | ||
| return sdkName.isExactName ? sdkName.name : normalizeName(sdkName.name, NameType.Property); |
There was a problem hiding this comment.
Can this be return normalizeSdkName(sdkName, NameType.Property); to avoid duplicate implementation?
| @@ -0,0 +1,285 @@ | |||
| import { afterAll, describe, expect, it } from "vitest"; | |||
There was a problem hiding this comment.
I suggest we move the tests to scenarios/ and use the Markdown pattern.
| ? "_" | ||
| : ""; | ||
| return `${internalModelPrefix}${normalizeName(namespacePrefix + type.name, nameType, true)}${unionSuffix}`; | ||
| const normalizedNamespacePrefix = |
There was a problem hiding this comment.
I am not sure what the expected behavior should be when enableModelNamespace is true. Do we need to add the namespace prefix for the models with exact name? But I believe this is a rare case, just want to confirm the behavior. Maybe we can add e unit test for it as well.
| nameType, | ||
| { shouldGuard: true }, | ||
| ); | ||
| return `${internalModelPrefix}${modelName}${unionSuffix}`; |
There was a problem hiding this comment.
Would it be clearer if we separate namespace prefix from model name as now they have different normalize logic:
const namespacePrefix = context.emitterOptions?.enableModelNamespace
? normalizeName(segments.join(""), nameType)
: "";
const modelName = normalizeSdkName(type, nameType, { shouldGuard: true }});
return `${internalModelPrefix}${namespacePrefix}${modelName}${unionSuffix}`;
| const groupName = normalizeName(rawGroupName, NameType.Property); | ||
| const groupName = normalizeName(prefixes[0] ?? "", NameType.Property); | ||
| const existProperty = clientClass.getProperties().filter((p) => { | ||
| return p.getName() === normalizeName(groupName, NameType.Property); |
There was a problem hiding this comment.
Maybe it's an issue with the existing code, not introduced by this PR - seems like we don't need to call normalizedName again because the groupName is already normalized.
I just tried with nested operation groups:
namespace Operations {
@route("/direct")
@get
op directOperation(): void;
namespace ChildOne {
@route("/child-one")
@get
op childOneOperation(): void;
}
namespace ChildTwo {
@route("/child-two")
@get
op childTwoOperation(): void;
}
}
#suppress "experimental-feature" "exact name test"
@@clientName(Operations, exact("my_group"));
And the generated code has three my_group properties in the classic client.
2 participants
Summary
exact()names across TypeScript clients, operations, parameters, models, properties, and enum membersValidation
mise exec -- pnpm formatmise exec -- pnpm lintmise exec -- pnpm --filter @azure-tools/typespec-ts buildmise exec -- pnpm --dir packages/typespec-ts unit-test(669 tests)mise exec -- pnpm --dir packages/typespec-ts test-next(254 tests)