-
Notifications
You must be signed in to change notification settings - Fork 553
improvement(client-core-interfaces): revise JsonDeserialized recursion #24785
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?
improvement(client-core-interfaces): revise JsonDeserialized recursion #24785
Conversation
… to objects Recursion can only happen with object types. Restrict to that and use `NonNullJsonObjectWith` to more accurately represent results.
Use `OpaqueJsonDeserialized` at recursion points for inexactly deserializable types instead of limited unrolling. Callers will need to use cast helper to resolve which will produce a repeatable pattern. Update tests with case for class with private data in a recursive situation.
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.
Pull Request Overview
This PR revises the JSON deserialization recursion handling by introducing the OpaqueJsonDeserialized wrapper at recursion points to enable a more repeatable and maintainable deserialization pattern.
- Updated test cases to validate recursion handling for classes with private data and for symbol, unknown, and handle recursion.
- Modified internal utility types to remove explicit recursion limits in favor of ancestor type checks and conditional wrapping.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
packages/common/core-interfaces/src/test/testValues.ts | Adds a test object type with optional recursion using a class with private data. |
packages/common/core-interfaces/src/test/jsonSerializable.spec.ts | Includes a new test case for private data in optional recursion. |
packages/common/core-interfaces/src/test/jsonDeserialized.spec.ts | Updates several test cases to validate the new OpaqueJsonDeserialized handling. |
packages/common/core-interfaces/src/exposedInternalUtilityTypes.ts | Refactors internal JSON deserialization utility types to use recursive ancestor type checks instead of a numeric recursion limit. |
Comments suppressed due to low confidence (2)
packages/common/core-interfaces/src/exposedInternalUtilityTypes.ts:1191
- The updated handling in JsonDeserializedRecursion uses the TypeUnderRecursion flag to switch to OpaqueJsonDeserialized. Consider clarifying the doc comment to detail how this flag affects recursion detection and when the wrapper is applied.
JsonDeserializedImpl<T, Controls, true>
packages/common/core-interfaces/src/test/jsonDeserialized.spec.ts:1073
- Ensure that the new behavior when wrapping recursive class instances in OpaqueJsonDeserialized is thoroughly tested for edge cases, particularly for deeply nested recursions.
it("class instance in object with optional recursion", () => {
Use
OpaqueJsonDeserialized
at recursion points for inexactly deserializable types instead of limited unrolling. Callers will need to use cast helper to resolve which will produce a repeatable pattern.Update tests with case for class with private data in a recursive situation.
Internally, ancestor types are now explicitly limited to
object
types.