From 5b42a1bd9b74773ffba1cdb80ded55090b832654 Mon Sep 17 00:00:00 2001 From: tadelesh Date: Tue, 18 Jun 2024 18:15:07 +0800 Subject: [PATCH 1/3] only expose top level client in `SdkPackage` --- .../src/package.ts | 5 +-- .../test/decorators.test.ts | 6 +-- .../test/package.test.ts | 43 ++++++++++--------- .../test/types/multipart-types.test.ts | 5 ++- 4 files changed, 30 insertions(+), 29 deletions(-) diff --git a/packages/typespec-client-generator-core/src/package.ts b/packages/typespec-client-generator-core/src/package.ts index 899a968724..6d9025a2a7 100644 --- a/packages/typespec-client-generator-core/src/package.ts +++ b/packages/typespec-client-generator-core/src/package.ts @@ -616,14 +616,11 @@ export function getSdkPackage< context.__clients = new Array>(); populateApiVersionInformation(context); const modelsAndEnums = diagnostics.pipe(getAllModelsWithDiagnostics(context)); - for (const client of listClients(context)) { - createSdkClientType(context, client); - } const crossLanguagePackageId = diagnostics.pipe(getCrossLanguagePackageId(context)); return { name: getClientNamespaceString(context)!, rootNamespace: getClientNamespaceString(context)!, - clients: Array.from(context.__clients.values()), + clients: listClients(context).map(c => diagnostics.pipe(createSdkClientType(context, c))), models: modelsAndEnums.filter((x): x is SdkModelType => x.kind === "model"), enums: modelsAndEnums.filter((x): x is SdkEnumType => x.kind === "enum"), diagnostics: diagnostics.diagnostics, diff --git a/packages/typespec-client-generator-core/test/decorators.test.ts b/packages/typespec-client-generator-core/test/decorators.test.ts index 9ebc95ff40..126c64323a 100644 --- a/packages/typespec-client-generator-core/test/decorators.test.ts +++ b/packages/typespec-client-generator-core/test/decorators.test.ts @@ -21,7 +21,7 @@ import { shouldGenerateConvenient, shouldGenerateProtocol, } from "../src/decorators.js"; -import { SdkMethodResponse, SdkOperationGroup, UsageFlags } from "../src/interfaces.js"; +import { SdkClientType, SdkHttpOperation, SdkMethodResponse, SdkOperationGroup, UsageFlags } from "../src/interfaces.js"; import { getCrossLanguageDefinitionId, getCrossLanguagePackageId } from "../src/public-utils.js"; import { getAllModels } from "../src/types.js"; import { SdkTestRunner, createSdkContextTestHelper, createSdkTestRunner } from "./test-host.js"; @@ -3284,7 +3284,7 @@ describe("typespec-client-generator-core: decorators", () => { ` ); const sdkPackage = runner.context.experimental_sdkPackage; - strictEqual(sdkPackage.clients.length, 2); + strictEqual(sdkPackage.clients.length, 1); const versioningClient = sdkPackage.clients.find((x) => x.name === "VersioningClient"); ok(versioningClient); strictEqual(versioningClient.methods.length, 2); @@ -3306,7 +3306,7 @@ describe("typespec-client-generator-core: decorators", () => { strictEqual(clientAccessor.name, "getInterfaceV2"); deepStrictEqual(clientAccessor.apiVersions, ["v2"]); - const interfaceV2 = sdkPackage.clients.find((x) => x.name === "InterfaceV2"); + const interfaceV2 = versioningClient.methods.find((x) => x.kind === "clientaccessor")?.response as SdkClientType; ok(interfaceV2); strictEqual(interfaceV2.methods.length, 1); diff --git a/packages/typespec-client-generator-core/test/package.test.ts b/packages/typespec-client-generator-core/test/package.test.ts index b9ec33b86b..ecae915ead 100644 --- a/packages/typespec-client-generator-core/test/package.test.ts +++ b/packages/typespec-client-generator-core/test/package.test.ts @@ -4,6 +4,7 @@ import { ApiKeyAuth, OAuth2Flow, Oauth2Auth } from "@typespec/http"; import { deepStrictEqual, ok, strictEqual } from "assert"; import { beforeEach, describe, it } from "vitest"; import { + SdkClientType, SdkCredentialParameter, SdkCredentialType, SdkEndpointParameter, @@ -577,10 +578,10 @@ describe("typespec-client-generator-core: package", () => { } `); const sdkPackage = runner.context.experimental_sdkPackage; - strictEqual(sdkPackage.clients.length, 2); + strictEqual(sdkPackage.clients.length, 1); const mainClient = sdkPackage.clients.find((c) => c.name === "TestServiceClient"); - const operationGroup = sdkPackage.clients.find((c) => c.name === "MyOperationGroup"); + const operationGroup = mainClient?.methods.find((c) => c.kind === "clientaccessor")?.response as SdkClientType; ok(mainClient && operationGroup); strictEqual(mainClient.methods.length, 1); @@ -620,12 +621,12 @@ describe("typespec-client-generator-core: package", () => { } `); const sdkPackage = runner.context.experimental_sdkPackage; - strictEqual(sdkPackage.clients.length, 4); + strictEqual(sdkPackage.clients.length, 1); - const mainClient = sdkPackage.clients.find((c) => c.name === "TestServiceClient"); - const fooClient = sdkPackage.clients.find((c) => c.name === "Foo"); - const fooBarClient = sdkPackage.clients.filter((c) => c.name === "Bar")![0]; - const barClient = sdkPackage.clients.filter((c) => c.name === "Bar")![1]; + const mainClient = sdkPackage.clients[0]; + const fooClient = mainClient.methods.find(m => m.kind === "clientaccessor" && m.name === "getFoo")?.response as SdkClientType; + const fooBarClient = fooClient.methods.find(m => m.kind === "clientaccessor")?.response as SdkClientType; + const barClient = mainClient.methods.find(m => m.kind === "clientaccessor" && m.name === "getBar")?.response as SdkClientType; ok(mainClient && fooClient && fooBarClient && barClient); strictEqual(mainClient.methods.length, 2); @@ -2449,12 +2450,10 @@ describe("typespec-client-generator-core: package", () => { ` ); const sdkPackage = runnerWithCore.context.experimental_sdkPackage; - strictEqual(sdkPackage.clients.length, 2); - const client = sdkPackage.clients.find((c) => c.initialization.access === "internal"); + strictEqual(sdkPackage.clients.length, 1); + const parentClient = sdkPackage.clients[0]; + const client = parentClient.methods.find(x => x.kind === "clientaccessor")?.response as SdkClientType; ok(client); - const parentClient = sdkPackage.clients.filter( - (c) => c.initialization.access === "public" - )[0]; strictEqual(client.methods.length, 2); // TEST GET STATUS @@ -2671,10 +2670,10 @@ describe("typespec-client-generator-core: package", () => { ` ); const sdkPackage = runnerWithCore.context.experimental_sdkPackage; - strictEqual(sdkPackage.clients.length, 2); + strictEqual(sdkPackage.clients.length, 1); strictEqual(sdkPackage.models.length, 1); strictEqual(sdkPackage.models[0].name, "Manufacturer"); - const widgetClient = sdkPackage.clients.find((c) => c.name === "Widgets"); + const widgetClient = sdkPackage.clients[0].methods.find(x => x.kind === "clientaccessor")?.response as SdkClientType; ok(widgetClient); strictEqual(widgetClient.initialization.properties.length, 3); strictEqual(widgetClient.initialization.access, "internal"); @@ -2953,7 +2952,8 @@ describe("typespec-client-generator-core: package", () => { sdkPackage.models.map((x) => x.name).sort(), ["CheckupCollectionWithNextLink", "Checkup", "PetStoreError", "CheckupUpdate"].sort() ); - const createOrUpdate = sdkPackage.clients[0].methods[0]; + const client = sdkPackage.clients[0].methods.find(x => x.kind === "clientaccessor")?.response as SdkClientType; + const createOrUpdate = client.methods[0]; strictEqual(createOrUpdate.kind, "basic"); strictEqual(createOrUpdate.name, "createOrUpdate"); strictEqual(createOrUpdate.parameters.length, 5); @@ -3048,7 +3048,9 @@ describe("typespec-client-generator-core: package", () => { const sdkPackage = runnerWithCore.context.experimental_sdkPackage; strictEqual(sdkPackage.models.length, 2); - const createOrReplace = sdkPackage.clients[0].methods[1]; + const client = sdkPackage.clients[0].methods.find(x => x.kind === "clientaccessor")?.response as SdkClientType; + + const createOrReplace = client.methods[1]; strictEqual(createOrReplace.kind, "basic"); strictEqual(createOrReplace.name, "createOrReplaceDataConnection"); strictEqual(createOrReplace.parameters.length, 5); @@ -3215,7 +3217,8 @@ describe("typespec-client-generator-core: package", () => { `); const sdkPackage = runner.context.experimental_sdkPackage; - strictEqual(sdkPackage.clients[0].methods[0].parameters[0].clientDefaultValue, "v2"); + const client = sdkPackage.clients[0].methods.find(x => x.kind === "clientaccessor")?.response as SdkClientType; + strictEqual(client.methods[0].parameters[0].clientDefaultValue, "v2"); }); it("default api version for operation is", async () => { @@ -3350,9 +3353,9 @@ function getServiceMethodOfClient( numMethods: number = 1, methodIndex: number = 0 ): SdkServiceMethod { - let client = sdkPackage.clients.filter((c) => c.initialization.access === "internal")[0]; - if (!client) { - client = sdkPackage.clients.filter((c) => c.initialization.access === "public")[0]; + let client = sdkPackage.clients[0]; + if (client.methods.some(x=>x.kind === "clientaccessor")) { + client = client.methods.find(x=>x.kind ==="clientaccessor")?.response as SdkClientType; } strictEqual(client.methods.length, numMethods); const method = client.methods[methodIndex]; diff --git a/packages/typespec-client-generator-core/test/types/multipart-types.test.ts b/packages/typespec-client-generator-core/test/types/multipart-types.test.ts index f4734e744f..bb84715e75 100644 --- a/packages/typespec-client-generator-core/test/types/multipart-types.test.ts +++ b/packages/typespec-client-generator-core/test/types/multipart-types.test.ts @@ -2,7 +2,7 @@ import { expectDiagnostics } from "@typespec/compiler/testing"; import { ok, strictEqual } from "assert"; import { beforeEach, describe, it } from "vitest"; -import { UsageFlags } from "../../src/interfaces.js"; +import { SdkClientType, SdkHttpOperation, UsageFlags } from "../../src/interfaces.js"; import { getAllModelsWithDiagnostics } from "../../src/types.js"; import { SdkTestRunner, createSdkTestRunner } from "../test-host.js"; @@ -208,7 +208,8 @@ describe("typespec-client-generator-core: multipart types", () => { upload(...WidgetForm): Widget; } `); - const formDataMethod = runner.context.experimental_sdkPackage.clients[0].methods[0]; + const client = runner.context.experimental_sdkPackage.clients[0].methods.find(x => x.kind === "clientaccessor")?.response as SdkClientType; + const formDataMethod = client.methods[0]; strictEqual(formDataMethod.kind, "basic"); strictEqual(formDataMethod.name, "upload"); strictEqual(formDataMethod.parameters.length, 3); From 2531557e0d95277d0c2475a24135d7133511e1de Mon Sep 17 00:00:00 2001 From: tadelesh Date: Tue, 18 Jun 2024 18:19:47 +0800 Subject: [PATCH 2/3] changelog --- .../client_hierarchy-2024-5-18-18-19-30.md | 7 ++++ .../src/package.ts | 2 +- .../test/decorators.test.ts | 11 ++++-- .../test/package.test.ts | 34 +++++++++++++------ .../test/types/multipart-types.test.ts | 4 ++- 5 files changed, 43 insertions(+), 15 deletions(-) create mode 100644 .chronus/changes/client_hierarchy-2024-5-18-18-19-30.md diff --git a/.chronus/changes/client_hierarchy-2024-5-18-18-19-30.md b/.chronus/changes/client_hierarchy-2024-5-18-18-19-30.md new file mode 100644 index 0000000000..e0e79f7795 --- /dev/null +++ b/.chronus/changes/client_hierarchy-2024-5-18-18-19-30.md @@ -0,0 +1,7 @@ +--- +changeKind: breaking +packages: + - "@azure-tools/typespec-client-generator-core" +--- + +only expose top level client in `SdkPackage` \ No newline at end of file diff --git a/packages/typespec-client-generator-core/src/package.ts b/packages/typespec-client-generator-core/src/package.ts index 6d9025a2a7..84ba697136 100644 --- a/packages/typespec-client-generator-core/src/package.ts +++ b/packages/typespec-client-generator-core/src/package.ts @@ -620,7 +620,7 @@ export function getSdkPackage< return { name: getClientNamespaceString(context)!, rootNamespace: getClientNamespaceString(context)!, - clients: listClients(context).map(c => diagnostics.pipe(createSdkClientType(context, c))), + clients: listClients(context).map((c) => diagnostics.pipe(createSdkClientType(context, c))), models: modelsAndEnums.filter((x): x is SdkModelType => x.kind === "model"), enums: modelsAndEnums.filter((x): x is SdkEnumType => x.kind === "enum"), diagnostics: diagnostics.diagnostics, diff --git a/packages/typespec-client-generator-core/test/decorators.test.ts b/packages/typespec-client-generator-core/test/decorators.test.ts index 126c64323a..c9a55d64da 100644 --- a/packages/typespec-client-generator-core/test/decorators.test.ts +++ b/packages/typespec-client-generator-core/test/decorators.test.ts @@ -21,7 +21,13 @@ import { shouldGenerateConvenient, shouldGenerateProtocol, } from "../src/decorators.js"; -import { SdkClientType, SdkHttpOperation, SdkMethodResponse, SdkOperationGroup, UsageFlags } from "../src/interfaces.js"; +import { + SdkClientType, + SdkHttpOperation, + SdkMethodResponse, + SdkOperationGroup, + UsageFlags, +} from "../src/interfaces.js"; import { getCrossLanguageDefinitionId, getCrossLanguagePackageId } from "../src/public-utils.js"; import { getAllModels } from "../src/types.js"; import { SdkTestRunner, createSdkContextTestHelper, createSdkTestRunner } from "./test-host.js"; @@ -3306,7 +3312,8 @@ describe("typespec-client-generator-core: decorators", () => { strictEqual(clientAccessor.name, "getInterfaceV2"); deepStrictEqual(clientAccessor.apiVersions, ["v2"]); - const interfaceV2 = versioningClient.methods.find((x) => x.kind === "clientaccessor")?.response as SdkClientType; + const interfaceV2 = versioningClient.methods.find((x) => x.kind === "clientaccessor") + ?.response as SdkClientType; ok(interfaceV2); strictEqual(interfaceV2.methods.length, 1); diff --git a/packages/typespec-client-generator-core/test/package.test.ts b/packages/typespec-client-generator-core/test/package.test.ts index ecae915ead..fcdff655f0 100644 --- a/packages/typespec-client-generator-core/test/package.test.ts +++ b/packages/typespec-client-generator-core/test/package.test.ts @@ -581,7 +581,8 @@ describe("typespec-client-generator-core: package", () => { strictEqual(sdkPackage.clients.length, 1); const mainClient = sdkPackage.clients.find((c) => c.name === "TestServiceClient"); - const operationGroup = mainClient?.methods.find((c) => c.kind === "clientaccessor")?.response as SdkClientType; + const operationGroup = mainClient?.methods.find((c) => c.kind === "clientaccessor") + ?.response as SdkClientType; ok(mainClient && operationGroup); strictEqual(mainClient.methods.length, 1); @@ -624,9 +625,14 @@ describe("typespec-client-generator-core: package", () => { strictEqual(sdkPackage.clients.length, 1); const mainClient = sdkPackage.clients[0]; - const fooClient = mainClient.methods.find(m => m.kind === "clientaccessor" && m.name === "getFoo")?.response as SdkClientType; - const fooBarClient = fooClient.methods.find(m => m.kind === "clientaccessor")?.response as SdkClientType; - const barClient = mainClient.methods.find(m => m.kind === "clientaccessor" && m.name === "getBar")?.response as SdkClientType; + const fooClient = mainClient.methods.find( + (m) => m.kind === "clientaccessor" && m.name === "getFoo" + )?.response as SdkClientType; + const fooBarClient = fooClient.methods.find((m) => m.kind === "clientaccessor") + ?.response as SdkClientType; + const barClient = mainClient.methods.find( + (m) => m.kind === "clientaccessor" && m.name === "getBar" + )?.response as SdkClientType; ok(mainClient && fooClient && fooBarClient && barClient); strictEqual(mainClient.methods.length, 2); @@ -2452,7 +2458,8 @@ describe("typespec-client-generator-core: package", () => { const sdkPackage = runnerWithCore.context.experimental_sdkPackage; strictEqual(sdkPackage.clients.length, 1); const parentClient = sdkPackage.clients[0]; - const client = parentClient.methods.find(x => x.kind === "clientaccessor")?.response as SdkClientType; + const client = parentClient.methods.find((x) => x.kind === "clientaccessor") + ?.response as SdkClientType; ok(client); strictEqual(client.methods.length, 2); @@ -2673,7 +2680,8 @@ describe("typespec-client-generator-core: package", () => { strictEqual(sdkPackage.clients.length, 1); strictEqual(sdkPackage.models.length, 1); strictEqual(sdkPackage.models[0].name, "Manufacturer"); - const widgetClient = sdkPackage.clients[0].methods.find(x => x.kind === "clientaccessor")?.response as SdkClientType; + const widgetClient = sdkPackage.clients[0].methods.find((x) => x.kind === "clientaccessor") + ?.response as SdkClientType; ok(widgetClient); strictEqual(widgetClient.initialization.properties.length, 3); strictEqual(widgetClient.initialization.access, "internal"); @@ -2952,7 +2960,8 @@ describe("typespec-client-generator-core: package", () => { sdkPackage.models.map((x) => x.name).sort(), ["CheckupCollectionWithNextLink", "Checkup", "PetStoreError", "CheckupUpdate"].sort() ); - const client = sdkPackage.clients[0].methods.find(x => x.kind === "clientaccessor")?.response as SdkClientType; + const client = sdkPackage.clients[0].methods.find((x) => x.kind === "clientaccessor") + ?.response as SdkClientType; const createOrUpdate = client.methods[0]; strictEqual(createOrUpdate.kind, "basic"); strictEqual(createOrUpdate.name, "createOrUpdate"); @@ -3048,7 +3057,8 @@ describe("typespec-client-generator-core: package", () => { const sdkPackage = runnerWithCore.context.experimental_sdkPackage; strictEqual(sdkPackage.models.length, 2); - const client = sdkPackage.clients[0].methods.find(x => x.kind === "clientaccessor")?.response as SdkClientType; + const client = sdkPackage.clients[0].methods.find((x) => x.kind === "clientaccessor") + ?.response as SdkClientType; const createOrReplace = client.methods[1]; strictEqual(createOrReplace.kind, "basic"); @@ -3217,7 +3227,8 @@ describe("typespec-client-generator-core: package", () => { `); const sdkPackage = runner.context.experimental_sdkPackage; - const client = sdkPackage.clients[0].methods.find(x => x.kind === "clientaccessor")?.response as SdkClientType; + const client = sdkPackage.clients[0].methods.find((x) => x.kind === "clientaccessor") + ?.response as SdkClientType; strictEqual(client.methods[0].parameters[0].clientDefaultValue, "v2"); }); @@ -3354,8 +3365,9 @@ function getServiceMethodOfClient( methodIndex: number = 0 ): SdkServiceMethod { let client = sdkPackage.clients[0]; - if (client.methods.some(x=>x.kind === "clientaccessor")) { - client = client.methods.find(x=>x.kind ==="clientaccessor")?.response as SdkClientType; + if (client.methods.some((x) => x.kind === "clientaccessor")) { + client = client.methods.find((x) => x.kind === "clientaccessor") + ?.response as SdkClientType; } strictEqual(client.methods.length, numMethods); const method = client.methods[methodIndex]; diff --git a/packages/typespec-client-generator-core/test/types/multipart-types.test.ts b/packages/typespec-client-generator-core/test/types/multipart-types.test.ts index bb84715e75..cd6ea53e17 100644 --- a/packages/typespec-client-generator-core/test/types/multipart-types.test.ts +++ b/packages/typespec-client-generator-core/test/types/multipart-types.test.ts @@ -208,7 +208,9 @@ describe("typespec-client-generator-core: multipart types", () => { upload(...WidgetForm): Widget; } `); - const client = runner.context.experimental_sdkPackage.clients[0].methods.find(x => x.kind === "clientaccessor")?.response as SdkClientType; + const client = runner.context.experimental_sdkPackage.clients[0].methods.find( + (x) => x.kind === "clientaccessor" + )?.response as SdkClientType; const formDataMethod = client.methods[0]; strictEqual(formDataMethod.kind, "basic"); strictEqual(formDataMethod.name, "upload"); From 1251f59535c0abf0aee213f8b4b55c36258fd98f Mon Sep 17 00:00:00 2001 From: tadelesh Date: Tue, 18 Jun 2024 23:11:36 +0800 Subject: [PATCH 3/3] remove useless client cache --- packages/typespec-client-generator-core/src/interfaces.ts | 1 - packages/typespec-client-generator-core/src/package.ts | 2 -- 2 files changed, 3 deletions(-) diff --git a/packages/typespec-client-generator-core/src/interfaces.ts b/packages/typespec-client-generator-core/src/interfaces.ts index d0e015161f..2bd1e8ae46 100644 --- a/packages/typespec-client-generator-core/src/interfaces.ts +++ b/packages/typespec-client-generator-core/src/interfaces.ts @@ -26,7 +26,6 @@ export interface SdkContext< > extends TCGCContext { emitContext: EmitContext; experimental_sdkPackage: SdkPackage; - __clients?: SdkClientType[]; } export interface SdkEmitterOptions { diff --git a/packages/typespec-client-generator-core/src/package.ts b/packages/typespec-client-generator-core/src/package.ts index 84ba697136..1813422ca2 100644 --- a/packages/typespec-client-generator-core/src/package.ts +++ b/packages/typespec-client-generator-core/src/package.ts @@ -573,7 +573,6 @@ function createSdkClientType< // eslint-disable-next-line deprecation/deprecation arm: client.kind === "SdkClient" ? client.arm : false, }; - context.__clients!.push(sdkClientType); return diagnostics.wrap(sdkClientType); } @@ -613,7 +612,6 @@ export function getSdkPackage< TServiceOperation extends SdkServiceOperation, >(context: SdkContext): SdkPackage { const diagnostics = createDiagnosticCollector(); - context.__clients = new Array>(); populateApiVersionInformation(context); const modelsAndEnums = diagnostics.pipe(getAllModelsWithDiagnostics(context)); const crossLanguagePackageId = diagnostics.pipe(getCrossLanguagePackageId(context));