More consistent schema metadata #25656
Conversation
github-actions
bot
added
area: dds
There was a problem hiding this comment.
Pull Request Overview
Updates SchemaFactory methods to consistently support node schema metadata options across all APIs. The PR adds an optional options parameter to array, map, and object methods in the base SchemaFactory class, and updates the corresponding record method in SchemaFactoryBeta, making metadata handling consistent with SchemaFactoryAlpha.
- Adds
TCustomMetadatageneric parameter and optionaloptionsparameter to named schema factory methods - Consolidates metadata handling by passing a unified
nodeOptionsobject to underlying schema creation functions - Updates all API report files to reflect the new method signatures with consistent metadata support
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/framework/fluid-framework/api-report/*.api.md | Updated method signatures to include optional metadata parameters |
| packages/dds/tree/api-report/*.api.md | Updated method signatures and type definitions for consistent metadata support |
| packages/dds/tree/src/simple-tree/api/schemaFactory*.ts | Added options parameters and refactored to use unified nodeOptions objects |
| packages/dds/tree/src/simple-tree/node-kinds//.ts | Updated schema creation functions to accept unified options objects |
| packages/dds/tree/src/test/simple-tree/core/treeNodeSchema.spec.ts | Updated test to use new API signature |
| .changeset/all-glasses-grab.md | Added changeset documenting the new consistent metadata support |
| /* ImplicitlyConstructable */ ImplicitlyConstructable, | ||
| /* Info */ T, | ||
| /* TConstructorExtra */ never, | ||
| /* TConstructorExtra */ undefined, |
There was a problem hiding this comment.
The change from never to undefined for TConstructorExtra should be documented. This affects the type system behavior and could impact consumers who rely on the previous never type.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Record nodes can be constructed with no constructor parameter: explicitly indicating that here saves on typecasting where this is used in schema factory beta.
| public static readonly identifierFieldKeys: readonly FieldKey[] = identifierFieldKeys; | ||
| public static readonly allowUnknownOptionalFields: boolean = allowUnknownOptionalFields; | ||
| public static readonly allowUnknownOptionalFields: boolean = | ||
| nodeOptions.allowUnknownOptionalFields ?? false; |
There was a problem hiding this comment.
Why is allowUnknownOptionalFields optional? Was this to avoid breaking an API or something?
There was a problem hiding this comment.
It is a setting nodes can opt into. We do not want to force every single schema declaration in existence to explicitly have to pass an options object and provide this property. That would be a breaking change, but also a really annoying API.
|
I broke out the non-public subset of this into #25685 to do first. After that, some work on defaults for options may be done before doing the public parts. |
## Description Expose schema metadata in more internal and beta API surfaces. This is a subset of #25656, but with the public SchemaFactory changes reverted.
|
🔗 Found some broken links! 💔 Run a link check locally to find them. See linkcheck output |
2 participants
Description
Make handling of node schema options like metadata more consistent across the various API layers.
Reviewer Guidance
The review process is outlined on this wiki page.