-
Notifications
You must be signed in to change notification settings - Fork 318
Changes to mutation node and canonicalization implementations. #8972
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
base: main
Are you sure you want to change the base?
Conversation
|
All changed packages have been documented.
Show changes
|
|
You can try these changes here
|
| import { getEncode, type MemberType, type Program, type Type } from "@typespec/compiler"; | ||
| import type { Typekit } from "@typespec/compiler/typekit"; | ||
| import { isHeader } from "@typespec/http"; | ||
| import type { HttpCanonicalization } from "./http-canonicalization-classes.js"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes here are because we need to figure out what the codec is before we construct a canonicalization (because the codec is used to determine the mutation key for a type).
Last week, @joheredi discovered that the name of the merge patch model was the name of the template rather than the mutated name of the type. When investigating this issue, I found that we did not handle updating mutated model properties' model linkage to the mutated model. Doing so led to uncovering a rather fundamental assumption that no longer worked.
Presently, within a subgraph, the only time you can have multiple mutation nodes for a type is when it is a reference mutation, keyed off the model property type that forms the reference. But if we need to have model properties be unique whenever the model is, then those properties need to be an exception too. And then what do we do with the types those model properties reference? Given a situation where A references B references C, do you get unique mutation nodes for A, B, C, A->B, B->C and A->B->C (i.e. the reference mutation is keyed off the possibly cloned member type)? or just A, B, C, A->B, B->C (i.e. the reference mutation is keyed off the source member type)? Something else? There just isn't a good answer here that generalizes.
So the only thing to do was to let the developer decide exactly how their mutation nodes are considered to be unique. Thus, the mutation node graph is changed:
With this out of the way, the notion of a subgraph and reference mutation has to be removed from the high-level Mutation nodes. Subgraphs are straight forward - we just use a suffix instead. To replace reference mutation nodes, we need to instead determine the mutation key for a referenced type, which may depend on the member types which reference it. The higher-level Mutations already have a notion similar to the mutation key - MutationOption's cache key. It made sense to align these concepts, so Mutations are also unique for a given (type, key) pair. But options is not sufficient because they are set by the caller, and we really want to be calculating the key for a given Scalar within a Scalar mutation.
So, a new static method is added to Mutations - mutationInfo. This method calculates the mutation key to use for a given type and member type references. This mutation key is then used to determine whether we need to construct a new Mutation or reuse an existing one. Within a Mutation constructor, MutationNodes are created by either using the mutation key directly, or adding a suffix to create multiple mutations. Since calculating the key may require expensive computation that will be useful for the eventually constructed mutation, mutationInfo can return additional info that is passed along to the constructor for the Mutation. So:
There is one additional problem, which is that when we are mutating a type and need to get the mutation for its connected types, we don't actually know what the mutation nodes will end up being. So we have introduced the concept of a
HalfEdgeto bothMutationandMutationNode. When we recurse to mutate connected types, we establish a half-edge that basically just says how to connect any new or existing nodes to the current node. MutationNodes were updated to handle connecting to already-mutated nodes which allows building up the mutation graph lazily.Additionally, two bugs are fixed: