Skip to content
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

[core] Improve documentation and implementation of Typekit, Mutator, and Realm #5149

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

witemple-msft
Copy link
Member

  • The experimental APIs are now documented with JSDoc that explains how to use the APIs.
  • The relationship between Realm and Typekit has been clarified and strengthened. A Typekit is now strongly bound to a realm on creation.
  • When using the default typekit, a default typekit realm is created for the current program if one does not exist already.
  • RealmKit has been removed (this was previously only used to statefully set/get the realm used by the default typekit), now realm is a readonly property of the typekit.
  • The default typekit $ has been merged with a function that gets a typekit bound to a specific realm ($(realm)) or bound to the default typekit realm of a specific program ($(program)) making it easier to work in specific program contexts (e.g. ProjectedProgram). The default typekit bound to a realm is cached in the Realm itself.
  • Each Program now has a "default typekit realm" that is created as needed when a typekit for that program is requested.
  • Additional typekit instances can still be created using createTypekit directly, bypassing the realm-caching behavior.
  • Mutators now use a typekit bound to their mutation realm, so that types cloned by the mutator are created within the realm and not within the typekit's default realm. (Previously, the mutator realm was unused within the mutator itself, only passed to the mutator functions).
  • The type of the mutator definition is now inferred from the definition of MutableType for accuracy. The previous definition was inaccurate and would fail to represent mutations of literal types correctly.
  • Added a test that mutation of literal types works as expected.
  • Worked out some module reference graph issues between defineKit and the typekit index that could sometimes lead to typekits being imported and invoked without the definitions of the typekit implementations being loaded.

@azure-sdk
Copy link
Collaborator

azure-sdk commented Nov 19, 2024

All changed packages have been documented.

  • @typespec/compiler
Show changes

@typespec/compiler - feature ✏️

Experimental: Improve Realm, Mutator, and Typekit implementations.,> ,> This change strongly binds a Realm and Typekit together, and changes mutators so that new types are cloned within the,> mutator's realm. The default Typekit now creates a default typekit realm for the current program, and a Typekit can be,> easily created to work in a specific Program or Realm as needed.

/**
* Mutate the type and recur, further mutating the type's children. This is the default behavior.
*/
MutateAndRecur = 0,
Copy link
Member

Choose a reason for hiding this comment

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

I get that recur is the proper verb form... but recur is also kind of a general term for "might happen again" which is a bit ambiguous and recurse is I think commonly used as a verb, but maybe I'm some kind of heathen.

Copy link
Member Author

Choose a reason for hiding this comment

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

Personally I've always heard and said "recur" and never "recurse" (e.g. clojure's recur operator) and didn't find a definition for "recurse" in the first dictionary I looked at (Merriam-Webster online), but I see now that many dictionaries do have a definition for it. In the context of formal functional programming, the relationship between a function and its recursive calls was always described to me as its "recurrence relation" and I think that "recursion" and "recurrence" are basically the same thing (after all, the reason it's called "recursion" is because you're defining a thing that recurs within its own definition.

I'm not married to the word "recur", so if you think "recurse" is better I wont object to reverting these back.

* causing parent navigation which in turn would mutate everything in that namespace.
* Mutate the type graph, allowing namespaces to be mutated.
*
* **Warning**: This function will likely mutate the entire type graph. Most TypeSpec types relate to namespaces
Copy link
Member

Choose a reason for hiding this comment

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

Might be good to add two things here: it will only mutate the entire type graph if you mutate namespaces, and if you don't want to mutate namespaces then you should use @ link mutateSubgraph.

Copy link
Member Author

Choose a reason for hiding this comment

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

It actually wont do that in its current implementation becuse mutateSubgraph does not follow namespace links. The implementation of child item following is pretty sparse.

If we have mutateSubgraph and mutateSubgraph with namespace traversal enabled, then it won't matter if you actually mutate a namespace, it will still follow namespace links in the visitor unless you visit those types and explicitly choose not to recur.

All this function actually does is allow you to pass a Namespace mutator at the type level. It doesn't do anything else differently. So the warning is optimistic for the future where we do have that behavioral difference.

I think at some point we will need to break mutateSubgraph out into an internal helper that allows you to say "yes I want to follow namespace links" or "no I dont want to follow namespace links".

Copy link
Member

@joheredi joheredi left a comment

Choose a reason for hiding this comment

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

Thanks for taking the time to also do JSDocs, super helpful!

@azure-sdk
Copy link
Collaborator

You can try these changes here

🛝 Playground 🌐 Website 📚 Next docs

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.

5 participants