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.

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

Conversation

joheredi
Copy link
Member

@joheredi joheredi commented Apr 8, 2025

Some tests were having issues in which caches were keeping information from previous programs. Switching to WeakMap and adding a map layer on program.

Copy link
Contributor

github-actions bot commented Apr 8, 2025

All changed packages have been documented.

  • @typespec/compiler
  • @typespec/http-client
Show changes

@typespec/compiler - internal ✏️

Update Typekit caches to use Program and WeekMap

@typespec/http-client-js - internal ✏️

Update Typekit caches to use Program and WeekMap

@typespec/http-client - internal ✏️

Update Typekit caches to use Program and WeekMap

@@ -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.

@azure-sdk
Copy link
Collaborator

You can try these changes here

🛝 Playground 🌐 Website 🛝 VSCode Extension

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