From 52e3bef218ad17040bb986f2fe5c7ffba3f70bfe Mon Sep 17 00:00:00 2001 From: Jose Manuel Heredia Hidalgo Date: Tue, 8 Apr 2025 20:26:49 +0000 Subject: [PATCH] Make Typekit cahces use Program and Weakmap --- ...typekit-cache-cleanup-2025-3-8-20-24-49.md | 9 ++++ .../src/experimental/typekit/kits/model.ts | 11 +++- .../test/scenarios/auth/bearer.md | 4 +- .../test/scenarios/auth/client_structure.md | 2 +- packages/http-client/src/client-library.ts | 20 +++++--- .../http-client/src/typekit/kits/client.ts | 50 ++++++++++++++++--- .../http-client/src/typekit/kits/operation.ts | 11 ++-- .../http-client/src/utils/client-helpers.ts | 13 ++++- 8 files changed, 93 insertions(+), 27 deletions(-) create mode 100644 .chronus/changes/joheredi-internal-typekit-cache-cleanup-2025-3-8-20-24-49.md diff --git a/.chronus/changes/joheredi-internal-typekit-cache-cleanup-2025-3-8-20-24-49.md b/.chronus/changes/joheredi-internal-typekit-cache-cleanup-2025-3-8-20-24-49.md new file mode 100644 index 00000000000..861a91fc578 --- /dev/null +++ b/.chronus/changes/joheredi-internal-typekit-cache-cleanup-2025-3-8-20-24-49.md @@ -0,0 +1,9 @@ +--- +changeKind: internal +packages: + - "@typespec/compiler" + - "@typespec/http-client-js" + - "@typespec/http-client" +--- + +Update Typekit caches to use Program and WeekMap \ No newline at end of file diff --git a/packages/compiler/src/experimental/typekit/kits/model.ts b/packages/compiler/src/experimental/typekit/kits/model.ts index e59a8417d97..7f39e6f61a0 100644 --- a/packages/compiler/src/experimental/typekit/kits/model.ts +++ b/packages/compiler/src/experimental/typekit/kits/model.ts @@ -5,6 +5,7 @@ import { getDiscriminatedUnionFromInheritance, } from "../../../core/helpers/discriminator-utils.js"; import { getDiscriminator } from "../../../core/intrinsic-type-state.js"; +import { Program } from "../../../core/program.js"; import type { Model, ModelIndexer, @@ -141,7 +142,7 @@ declare module "../define-kit.js" { interface Typekit extends TypekitExtension {} } -const spreadCache = new Map(); +const _spreadCache = new WeakMap>(); defineKit({ model: { create(desc) { @@ -172,6 +173,7 @@ defineKit({ return getEffectiveModelType(this.program, model, filter); }, getSpreadType(model) { + const spreadCache = getSpreadCache(this.program); if (spreadCache.has(model)) { return spreadCache.get(model); } @@ -255,3 +257,10 @@ defineKit({ }, }, }); + +function getSpreadCache(program: Program): WeakMap { + if (!_spreadCache.has(program)) { + _spreadCache.set(program, new WeakMap()); + } + return _spreadCache.get(program)!; +} diff --git a/packages/http-client-js/test/scenarios/auth/bearer.md b/packages/http-client-js/test/scenarios/auth/bearer.md index 800db6476af..31e93735504 100644 --- a/packages/http-client-js/test/scenarios/auth/bearer.md +++ b/packages/http-client-js/test/scenarios/auth/bearer.md @@ -26,7 +26,7 @@ The client signature should include a positional parameter for credential of typ export class TestClient { #context: TestClientContext; - constructor(endpoint: string, credential: BasicCredential, options?: TestClientOptions) { + constructor(endpoint: string, credential: BearerTokenCredential, options?: TestClientOptions) { this.#context = createTestClientContext(endpoint, credential, options); } async valid(options?: ValidOptions) { @@ -42,7 +42,7 @@ The client context should setup the pipeline to use the credential in the Author ```ts src/api/testClientContext.ts function createTestClientContext export function createTestClientContext( endpoint: string, - credential: BasicCredential, + credential: BearerTokenCredential, options?: TestClientOptions, ): TestClientContext { const params: Record = { diff --git a/packages/http-client-js/test/scenarios/auth/client_structure.md b/packages/http-client-js/test/scenarios/auth/client_structure.md index e52bdc143c9..4466d44b04f 100644 --- a/packages/http-client-js/test/scenarios/auth/client_structure.md +++ b/packages/http-client-js/test/scenarios/auth/client_structure.md @@ -27,7 +27,7 @@ export class MyApiClient { #context: MyApiClientContext; fooClient: FooClient; barClient: BarClient; - constructor(endpoint: string, credential: BasicCredential, options?: MyApiClientOptions) { + constructor(endpoint: string, credential: BearerTokenCredential, options?: MyApiClientOptions) { this.#context = createMyApiClientContext(endpoint, credential, options); this.fooClient = new FooClient(endpoint, credential, options); this.barClient = new BarClient(endpoint, credential, options); diff --git a/packages/http-client/src/client-library.ts b/packages/http-client/src/client-library.ts index 88ea03da556..6dd99b0d512 100644 --- a/packages/http-client/src/client-library.ts +++ b/packages/http-client/src/client-library.ts @@ -72,7 +72,7 @@ function getEffectiveClient(program: Program, namespace: Namespace): InternalCli return undefined; } -const operationClientMap = new Map>(); +const _operationClientMap = new WeakMap>(); export function createClientLibrary( program: Program, @@ -80,10 +80,6 @@ export function createClientLibrary( ): ClientLibrary { const $ = unsafe_$(program); - if (!operationClientMap.has(program)) { - operationClientMap.set(program, new Map()); - } - let topLevel: InternalClient[] = []; const dataTypes = new Set(); @@ -114,11 +110,13 @@ export function createClientLibrary( topLevelClients.push(client); } + const operationClientMap = getOperationClientMapping(program); + return { topLevel: topLevelClients, dataTypes: Array.from(dataTypes), getClientForOperation(operation: HttpOperation) { - return operationClientMap.get(program)?.get(operation); + return operationClientMap.get(operation); }, }; } @@ -133,6 +131,7 @@ function visitClient( dataTypes: Set, options?: VisitClientOptions, ): Client { + const operationClientMap = getOperationClientMapping(program); const $ = unsafe_$(program); // First create a partial `Client` object. // We’ll fill in subClients *after* we have `c`. @@ -153,7 +152,7 @@ function visitClient( // Now store the prepared operations currentClient.operations = $.client.listHttpOperations(client).map((o) => { - operationClientMap.get(program)?.set(o, currentClient); + operationClientMap.set(o, currentClient); return { client: currentClient, @@ -179,3 +178,10 @@ function visitClient( return currentClient; } + +function getOperationClientMapping(program: Program): Map { + if (!_operationClientMap.has(program)) { + _operationClientMap.set(program, new Map()); + } + return _operationClientMap.get(program)!; +} diff --git a/packages/http-client/src/typekit/kits/client.ts b/packages/http-client/src/typekit/kits/client.ts index 71f701f3e51..dd7533dadfd 100644 --- a/packages/http-client/src/typekit/kits/client.ts +++ b/packages/http-client/src/typekit/kits/client.ts @@ -6,6 +6,7 @@ import { Namespace, NoTarget, Operation, + Program, } from "@typespec/compiler"; import { $, defineKit } from "@typespec/compiler/experimental/typekit"; import { @@ -93,8 +94,12 @@ function getClientName(name: string): string { return name.endsWith("Client") ? name : `${name}Client`; } -export const clientCache = new Map(); -export const clientOperationCache = new Map(); +export const _clientCache = new WeakMap>(); +export const _clientOperationMapping = new WeakMap< + Program, + WeakMap +>(); +export const _operationClientCache = new WeakMap>(); defineKit({ client: { @@ -119,6 +124,7 @@ defineKit({ return clients; }, getClient(namespace) { + const clientCache = getClientCache(this.program); if (!namespace) { reportDiagnostic(this.program, { code: "undefined-namespace-for-client", @@ -153,8 +159,9 @@ defineKit({ return client.type.kind === "Namespace"; }, listHttpOperations(client) { - if (clientOperationCache.has(client)) { - return clientOperationCache.get(client)!; + const clientOperationMapping = getClientOperationMapping(this.program); + if (clientOperationMapping.has(client)) { + return clientOperationMapping.get(client)!; } if (client.type.kind === "Interface" && isTemplateDeclaration(client.type)) { @@ -173,8 +180,14 @@ defineKit({ operations.push(clientOperation); } - const httpOperations = operations.map((o) => this.httpOperation.get(o)); - clientOperationCache.set(client, httpOperations); + const operationClientMapping = getOperationClientMapping(this.program); + + const httpOperations = operations.map((o) => { + const httpOp = this.httpOperation.get(o); + operationClientMapping.set(httpOp, client); + return httpOp; + }); + clientOperationMapping.set(client, httpOperations); return httpOperations; }, @@ -283,3 +296,28 @@ defineKit({ }, }, }); + +export function getOperationClientMapping( + program: Program, +): WeakMap { + if (!_operationClientCache.has(program)) { + _operationClientCache.set(program, new WeakMap()); + } + return _operationClientCache.get(program)!; +} + +export function getClientOperationMapping( + program: Program, +): WeakMap { + if (!_clientOperationMapping.has(program)) { + _clientOperationMapping.set(program, new WeakMap()); + } + return _clientOperationMapping.get(program)!; +} + +function getClientCache(program: Program): WeakMap { + if (!_clientCache.has(program)) { + _clientCache.set(program, new WeakMap()); + } + return _clientCache.get(program)!; +} diff --git a/packages/http-client/src/typekit/kits/operation.ts b/packages/http-client/src/typekit/kits/operation.ts index 5e35c907d43..a170585d172 100644 --- a/packages/http-client/src/typekit/kits/operation.ts +++ b/packages/http-client/src/typekit/kits/operation.ts @@ -3,7 +3,7 @@ import { defineKit } from "@typespec/compiler/experimental/typekit"; import { HttpOperation } from "@typespec/http"; import { InternalClient as Client } from "../../interfaces.js"; import { getConstructors } from "../../utils/client-helpers.js"; -import { clientOperationCache } from "./client.js"; +import { getOperationClientMapping } from "./client.js"; import { AccessKit, getAccess, getName, NameKit } from "./utils.js"; export interface SdkOperationKit extends NameKit, AccessKit { @@ -46,13 +46,8 @@ declare module "@typespec/compiler/experimental/typekit" { defineKit({ operation: { getClient(operation) { - for (const [client, operations] of clientOperationCache.entries()) { - if (operations.includes(operation)) { - return client; - } - } - - return undefined; + const operationClientMapping = getOperationClientMapping(this.program); + return operationClientMapping.get(operation); }, getOverloads(client, operation) { if (operation.name === "constructor") { diff --git a/packages/http-client/src/utils/client-helpers.ts b/packages/http-client/src/utils/client-helpers.ts index 90e03d142f6..0238ee63816 100644 --- a/packages/http-client/src/utils/client-helpers.ts +++ b/packages/http-client/src/utils/client-helpers.ts @@ -1,12 +1,13 @@ import { Refkey, refkey } from "@alloy-js/core"; -import { ModelProperty, Operation, StringLiteral, Type } from "@typespec/compiler"; +import { ModelProperty, Operation, Program, StringLiteral, Type } from "@typespec/compiler"; import { $ } from "@typespec/compiler/experimental/typekit"; import { getHttpService, resolveAuthentication } from "@typespec/http"; import { InternalClient } from "../interfaces.js"; import { authSchemeSymbol, credentialSymbol } from "../types/credential-symbol.js"; import { getStringValue, getUniqueTypes } from "./helpers.js"; -const credentialCache = new Map(); +const credentialCache = new WeakMap>(); + export function getCredentialParameter(client: InternalClient): ModelProperty | undefined { const [httpService] = getHttpService($.program, client.service); @@ -19,6 +20,7 @@ export function getCredentialParameter(client: InternalClient): ModelProperty | }); const cacheKey = getCredRefkey(credTypes); + const credentialCache = getCredentialCache($.program); if (credentialCache.has(cacheKey)) { return credentialCache.get(cacheKey)!; @@ -188,3 +190,10 @@ export function createBaseConstructor( returnType: $.program.checker.voidType, }); } + +function getCredentialCache(program: Program): WeakMap { + if (!credentialCache.has(program)) { + credentialCache.set(program, new WeakMap()); + } + return credentialCache.get(program)!; +}