Skip to content

Make Typekit cahces use Program and Weakmap #6911

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Make Typekit cahces use Program and Weakmap #6911

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .chronus/changes/joheredi-internal-typekit-cache-cleanup-2025-3-8-20-24-49.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
changeKind: internal
packages:
- "@typespec/compiler"
- "@typespec/http-client-js"
- "@typespec/http-client"
---

Update Typekit caches to use Program and WeekMap
11 changes: 10 additions & 1 deletion packages/compiler/src/experimental/typekit/kits/model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -141,7 +142,7 @@ declare module "../define-kit.js" {
interface Typekit extends TypekitExtension {}
}

const spreadCache = new Map<Model, Model>();
const _spreadCache = new WeakMap<Program, WeakMap<Model, Model>>();
defineKit<TypekitExtension>({
model: {
create(desc) {
Expand Down Expand Up @@ -172,6 +173,7 @@ defineKit<TypekitExtension>({
return getEffectiveModelType(this.program, model, filter);
},
getSpreadType(model) {
Copy link
Member

Choose a reason for hiding this comment

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

very confused by the purpose of this typekit

Copy link
Member Author

Choose a reason for hiding this comment

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

If I recall correctly this was so that you can know from a model Foo if what type is spread within it.

model Bar {
  value: int32;
}
model Foo {
   ...Bar;
   name: string;
}

getSpreadType(Foo) -> Bar

If there is a better way to do this, I'm happy to switch. I think this was useful in the AdditionalProperties scenario

Copy link
Member

Choose a reason for hiding this comment

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

I mean if it was that I would kinda understand but here it just seems to be creating a new record/array model with the same indexer which is not doing what you describe(as far as I understand it).

For resolving the list of models spread you can just check the sourceModels property not sure we need a typekit for that.

const spreadCache = getSpreadCache(this.program);
if (spreadCache.has(model)) {
return spreadCache.get(model);
}
Expand Down Expand Up @@ -255,3 +257,10 @@ defineKit<TypekitExtension>({
},
},
});

function getSpreadCache(program: Program): WeakMap<Model, Model> {
if (!_spreadCache.has(program)) {
_spreadCache.set(program, new WeakMap());
}
return _spreadCache.get(program)!;
}
4 changes: 2 additions & 2 deletions packages/http-client-js/test/scenarios/auth/bearer.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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<string, any> = {
Expand Down
2 changes: 1 addition & 1 deletion packages/http-client-js/test/scenarios/auth/client_structure.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
20 changes: 13 additions & 7 deletions packages/http-client/src/client-library.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,18 +72,14 @@ function getEffectiveClient(program: Program, namespace: Namespace): InternalCli
return undefined;
}

const operationClientMap = new Map<Program, Map<HttpOperation, Client>>();
const _operationClientMap = new WeakMap<Program, Map<HttpOperation, Client>>();

export function createClientLibrary(
program: Program,
options: CreateClientLibraryOptions = {},
): ClientLibrary {
const $ = unsafe_$(program);

if (!operationClientMap.has(program)) {
operationClientMap.set(program, new Map<HttpOperation, Client>());
}

let topLevel: InternalClient[] = [];
const dataTypes = new Set<Model | Union | Enum>();

Expand Down Expand Up @@ -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);
},
};
}
Expand All @@ -133,6 +131,7 @@ function visitClient(
dataTypes: Set<Model | Union | Enum>,
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`.
Expand All @@ -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,
Expand All @@ -179,3 +178,10 @@ function visitClient(

return currentClient;
}

function getOperationClientMapping(program: Program): Map<HttpOperation, Client> {
if (!_operationClientMap.has(program)) {
_operationClientMap.set(program, new Map<HttpOperation, Client>());
}
return _operationClientMap.get(program)!;
}
50 changes: 44 additions & 6 deletions packages/http-client/src/typekit/kits/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
Namespace,
NoTarget,
Operation,
Program,
} from "@typespec/compiler";
import { $, defineKit } from "@typespec/compiler/experimental/typekit";
import {
Expand Down Expand Up @@ -93,8 +94,12 @@ function getClientName(name: string): string {
return name.endsWith("Client") ? name : `${name}Client`;
}

export const clientCache = new Map<Namespace | Interface, InternalClient>();
export const clientOperationCache = new Map<InternalClient, HttpOperation[]>();
export const _clientCache = new WeakMap<Program, WeakMap<Namespace | Interface, InternalClient>>();
export const _clientOperationMapping = new WeakMap<
Program,
WeakMap<InternalClient, HttpOperation[]>
>();
export const _operationClientCache = new WeakMap<Program, WeakMap<HttpOperation, InternalClient>>();

defineKit<TypekitExtension>({
client: {
Expand All @@ -119,6 +124,7 @@ defineKit<TypekitExtension>({
return clients;
},
getClient(namespace) {
const clientCache = getClientCache(this.program);
if (!namespace) {
reportDiagnostic(this.program, {
code: "undefined-namespace-for-client",
Expand Down Expand Up @@ -153,8 +159,9 @@ defineKit<TypekitExtension>({
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)) {
Expand All @@ -173,8 +180,14 @@ defineKit<TypekitExtension>({
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;
},
Expand Down Expand Up @@ -283,3 +296,28 @@ defineKit<TypekitExtension>({
},
},
});

export function getOperationClientMapping(
program: Program,
): WeakMap<HttpOperation, InternalClient> {
if (!_operationClientCache.has(program)) {
_operationClientCache.set(program, new WeakMap());
}
return _operationClientCache.get(program)!;
}

export function getClientOperationMapping(
program: Program,
): WeakMap<InternalClient, HttpOperation[]> {
if (!_clientOperationMapping.has(program)) {
_clientOperationMapping.set(program, new WeakMap());
}
return _clientOperationMapping.get(program)!;
}

function getClientCache(program: Program): WeakMap<Namespace | Interface, InternalClient> {
if (!_clientCache.has(program)) {
_clientCache.set(program, new WeakMap());
}
return _clientCache.get(program)!;
}
11 changes: 3 additions & 8 deletions packages/http-client/src/typekit/kits/operation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Operation>, AccessKit<Operation> {
Expand Down Expand Up @@ -46,13 +46,8 @@ declare module "@typespec/compiler/experimental/typekit" {
defineKit<SdkKit>({
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") {
Expand Down
13 changes: 11 additions & 2 deletions packages/http-client/src/utils/client-helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<Refkey, ModelProperty>();
const credentialCache = new WeakMap<Program, WeakMap<Refkey, ModelProperty>>();

export function getCredentialParameter(client: InternalClient): ModelProperty | undefined {
const [httpService] = getHttpService($.program, client.service);

Expand All @@ -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)!;
Expand Down Expand Up @@ -188,3 +190,10 @@ export function createBaseConstructor(
returnType: $.program.checker.voidType,
});
}

function getCredentialCache(program: Program): WeakMap<Refkey, ModelProperty> {
if (!credentialCache.has(program)) {
credentialCache.set(program, new WeakMap());
}
return credentialCache.get(program)!;
}
Loading