-
Notifications
You must be signed in to change notification settings - Fork 562
Support "delete" keyword for object nodes #25738
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
Conversation
noencke
left a comment
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.
We should document this behavior somewhere. I think it would go on the docs for the TreeObjectNode type. Make sure this new behavior is documented and doesn't conflict with any existing docs.
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 adds support for using the delete operator on optional fields in ObjectNodes, making it functionally equivalent to assigning undefined to those fields.
Key Changes:
- Replaced error-throwing
deletePropertyhandler with implementation that callsapplyFieldChangewithundefined - Refactored field assignment logic into a new
applyFieldChangehelper function - Updated test from expecting an error to validating the delete behavior
- Added documentation explaining field assignment and deletion semantics
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/dds/tree/src/simple-tree/node-kinds/object/objectNode.ts | Implements delete support via new applyFieldChange helper and updated proxy deleteProperty handler |
| packages/dds/tree/src/test/simple-tree/node-kinds/object/objectNode.spec.ts | Updates test to validate delete behavior instead of expecting error |
| .changeset/fifty-crabs-add.md | Documents the new feature for changelog |
| : (targetToProxy.get(from.node) ?? fail("missing proxy")); | ||
| const inner = getInnerNode(proxy); | ||
| const storedSchema = inner.context.schema.nodeSchema.get(brand(schema.identifier)); | ||
| assert(storedSchema instanceof ObjectNodeStoredSchema, "Expected ObjectNodeStoredSchema"); |
Copilot
AI
Oct 29, 2025
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.
When writing asserts (from @fluidframework/core-utils), please use a string literal for the error message, not a hex assert code. This instruction should only apply to newly added asserts, not existing ones.
Description
This PR adds support for the "delete" keyword in object nodes.
Breaking Changes
None