Skip to content

Name collisions from structurally named schema now error #24707

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

Merged
merged 13 commits into from
May 27, 2025

Conversation

CraigMacomber
Copy link
Contributor

Description

See changeset.

Breaking Changes

Some malformed schema which previously behaved incorrectly will now err.

Reviewer Guidance

The review process is outlined on this wiki page.

@Copilot Copilot AI review requested due to automatic review settings May 23, 2025 21:47
@CraigMacomber CraigMacomber requested review from a team as code owners May 23, 2025 21:47
@github-actions github-actions bot added area: dds Issues related to distributed data structures area: dds: tree base: main PRs targeted against main branch changeset-present labels May 23, 2025
Copy link
Contributor

@Copilot Copilot AI left a 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 enforces unique structurally named schemas by comparing input and existing definitions and throws a UsageError on mismatch.

  • Introduces a shared helper (getStructuralType) in schemaFactory.ts to validate and cache structural types for arrays and maps.
  • Adds unit tests (schemaFactory.spec.ts) for single- and multi-type collisions to ensure errors are thrown on mismatches.
  • Adds a changelog entry (.changeset/social-bears-deny.md) documenting the breaking change.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
packages/dds/tree/src/test/simple-tree/api/schemaFactory.spec.ts Adds tests for single and multi-type schema collisions.
packages/dds/tree/src/simple-tree/api/schemaFactory.ts Implements getStructuralType to detect and error on collisions.
.changeset/social-bears-deny.md Documents the new error behavior for structurally named schemas.
Comments suppressed due to low confidence (1)

packages/dds/tree/src/simple-tree/api/schemaFactory.ts:994

  • [nitpick] The variable name same is generic; consider renaming it to something more descriptive like typesMatch or schemasEqual to clarify its role in the comparison.
const same = compareSets({ a: inputTypes, b: outputTypes });

@CraigMacomber CraigMacomber enabled auto-merge (squash) May 23, 2025 23:07
@CraigMacomber CraigMacomber disabled auto-merge May 27, 2025 16:57
@CraigMacomber CraigMacomber merged commit a343f04 into microsoft:main May 27, 2025
32 checks passed
@CraigMacomber CraigMacomber deleted the structurallyCollide branch May 27, 2025 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: dds: tree area: dds Issues related to distributed data structures base: main PRs targeted against main branch changeset-present
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants