Shared Tree: Add allowed types object with staged schema upgrade information to SimpleSchema#25770
Conversation
…ests yet and I might be missing members.
…ewCompatibility mode.
…reful serialization of each type, though I think this safer and more durable than a more generic approach.
…owedTypesIdentifiers with allowedTypesInfo.
- Regenerated API files.
- Regenerated API Extractor output.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the schema representation to replace allowedTypesIdentifiers (a ReadonlySet<string>) with simpleAllowedTypes (a ReadonlyMap<string, SimpleAllowedTypes>) throughout the codebase. This change enables tracking additional metadata about allowed types, specifically whether a type is part of a staged schema upgrade.
Key changes:
- Introduces a new
SimpleAllowedTypesinterface to track metadata likeisStagedflag - Updates all schema interfaces to use
simpleAllowedTypesinstead ofallowedTypesIdentifiers - Adds new methods (
evaluateSimpleAllowedTypes) to compute the new map-based representation
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/dds/tree/src/simple-tree/simpleSchema.ts | Defines the new SimpleAllowedTypes interface and updates schema interfaces to use it |
| packages/dds/tree/src/simple-tree/core/allowedTypes.ts | Adds evaluateSimpleAllowedTypes() method to compute the map representation |
| packages/dds/tree/src/simple-tree/fieldSchema.ts | Implements simpleAllowedTypes getter in FieldSchemaAlpha |
| packages/dds/tree/src/simple-tree/node-kinds/array/arrayNode.ts | Adds lazy evaluation of simpleAllowedTypes for array schemas |
| packages/dds/tree/src/simple-tree/node-kinds/map/mapNode.ts | Adds lazy evaluation of simpleAllowedTypes for map schemas |
| packages/dds/tree/src/simple-tree/node-kinds/record/recordNode.ts | Adds lazy evaluation of simpleAllowedTypes for record schemas |
| packages/dds/tree/src/simple-tree/toStoredSchema.ts | Updates conversion logic to extract identifiers from the new map structure |
| packages/dds/tree/src/simple-tree/api/simpleSchemaToJsonSchema.ts | Updates JSON schema conversion to work with the new map structure |
| packages/dds/tree/src/simple-tree/api/viewSchemaToSimpleSchema.ts | Updates schema copying to use simpleAllowedTypes |
| packages/dds/tree/src/simple-tree/api/schemaFromSimple.ts | Updates schema generation from simple schemas to extract keys from the map |
| packages/dds/tree/src/shared-tree/sharedTree.ts | Adds helper function to build simpleAllowedTypes for stored schemas |
| packages/framework/tree-agent/src/typeGeneration.ts | Updates type generation to work with the new map structure |
| packages/framework/ai-collab/src/explicit-strategy/typeGeneration.ts | Updates AI collaboration type generation |
| packages/tools/devtools/devtools-core/src/data-visualization/*.ts | Updates visualization code to work with the new structure |
| Test files | Updates test expectations to use map-based representation |
| API report files | Documents the public API changes |
| * True if this schema is included as a {@link SchemaStaticsAlpha.staged | staged} schema upgrade, | ||
| * allowing the view schema be compatible with stored schema with (post upgrade) or without it (pre-upgrade). | ||
| * New documents and schema upgrades will omit any staged schema. | ||
| * | ||
| * Undefined if derived from a stored schema. |
There was a problem hiding this comment.
The fact that this differs between stored and view schema gives me some pause. I wonder if (at least longer term), we want slightly different variants of simple schema for stored vs view schema? What other differences are going to creep in over time here?
But for now this is probably fine. It is an alpha API. Maybe worth a quick @privateRemarks note about future considerations?
Josmithr
left a comment
There was a problem hiding this comment.
Left a couple of notes, but overall looks good to me.
…h is beta). - PR feedback.
Co-authored-by: Noah Encke <78610362+noencke@users.noreply.github.com>
…n/FluidFramework into simple-allowed-types
Josmithr
left a comment
There was a problem hiding this comment.
Looks like your API reports are out of date. Happy to re-approve once those have been updated :)
packages/dds/tree/src/test/simple-tree/api/simpleSchemaToJsonSchema.spec.ts
Show resolved
Hide resolved
…preserve view-specific attributes.
|
🔗 Found some broken links! 💔 Run a link check locally to find them. See linkcheck output |
…rmation to SimpleSchema (microsoft#25770) This PR removes the `allowedTypesIdentifiers` Set from Simple Schema types and replaces it with a Map keyed on the identifier. Additionally, `isStaged` has been added to reflect whether the allowed types have associated schema upgrades. ## Breaking Changes This change breaks a few alpha APIs in SimpleSchema: - `SimpleFieldSchema.allowedTypesIdentifiers` - `SimpleRecordNodeSchema.allowedTypesIdentifiers` - `SimpleMapNodeSchema.allowedTypesIdentifiers` - `SimpleArrayNodeSchema.allowedTypesIdentifiers` --------- Co-authored-by: Noah Encke <78610362+noencke@users.noreply.github.com>
This PR removes the
allowedTypesIdentifiersSet from Simple Schema types and replaces it with a Map keyed on the identifier. Additionally,isStagedhas been added to reflect whether the allowed types have associated schema upgrades.Breaking Changes
This change breaks a few alpha APIs in SimpleSchema:
SimpleFieldSchema.allowedTypesIdentifiersSimpleRecordNodeSchema.allowedTypesIdentifiersSimpleMapNodeSchema.allowedTypesIdentifiersSimpleArrayNodeSchema.allowedTypesIdentifiersReviewer Guidance
simpleAllowedTypesfields. I wanted to name this fieldallowedTypesto match the existing convention, but it conflicts with an existing field in the case ofSimpleFieldSchema.