Skip to content
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

Add scopedFactory #23987

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

CraigMacomber
Copy link
Contributor

Description

Add SchemaFactory.scopedFactory

Reviewer Guidance

The review process is outlined on this wiki page.

@Copilot Copilot bot review requested due to automatic review settings March 5, 2025 00:49
@CraigMacomber CraigMacomber requested review from a team as code owners March 5, 2025 00:49
@github-actions github-actions bot added area: dds Issues related to distributed data structures area: dds: tree area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct changeset-present public api change Changes to a public API base: main PRs targeted against main branch labels Mar 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This PR adds a new method, SchemaFactory.scopedFactory, to simplify creating a new SchemaFactory with a nested scope string, enabling better organization and collision prevention for related schemas.

  • Introduces the scopedFactory method in the SchemaFactory class for handling nested scopes.
  • Updates tests to use the new scopedFactory method instead of manually concatenating scope strings.
  • Updates API report files to document the new scopedFactory method.

Reviewed Changes

File Description
.changeset/grumpy-crabs-hide.md Specifies version bumps and changelog entry for the new feature.
packages/dds/tree/src/simple-tree/api/schemaFactory.ts Adds the scopedFactory method implementation with proper JSDoc comments.
packages/dds/tree/src/test/simple-tree/api/schemaCreationUtilities.spec.ts Updates tests to utilize the new scopedFactory method.
Various API report files Documents the new scopedFactory method across different API versions.

Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.

Copy link
Contributor

github-actions bot commented Mar 5, 2025

🔗 Found some broken links! 💔

Run a link check locally to find them. See
https://github.com/microsoft/FluidFramework/wiki/Checking-for-broken-links-in-the-documentation for more information.

linkcheck output


> [email protected] ci:check-links /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test "npm run serve -- --no-open" 3000 check-links

1: starting server using command "npm run serve -- --no-open"
and when url "[ 'http://127.0.0.1:3000' ]" is responding with HTTP status code 200
running tests using command "npm run check-links"


> [email protected] serve
> docusaurus serve --no-open

[SUCCESS] Serving "build" directory at: http://localhost:3000/

> [email protected] check-links
> linkcheck http://localhost:3000 --skip-file skipped-urls.txt

Crawling...

http://localhost:3000/docs/data-structures/tree
- (36:97) 'the Shar..' => http://localhost:3000/docs/api/tree (HTTP 404)

http://localhost:3000/docs/data-structures/tree/
- (36:97) 'the Shar..' => http://localhost:3000/docs/api/tree (HTTP 404)

http://localhost:3000/docs/data-structures/tree/schema-definition
- (30:128) 'SharedTr..' => http://localhost:3000/docs/api/tree (HTTP 404)

http://localhost:3000/docs/start/tree-start
- (44:4) 'the API ..' => http://localhost:3000/docs/api/tree/schemafactory-class (HTTP 404)
- (61:7) 'the API' => http://localhost:3000/docs/api/tree/treechangeevents-interface (HTTP 404)


Stats:
  158264 links
    1304 destination URLs
    1535 URLs ignored
       0 warnings
       3 errors

 ELIFECYCLE  Command failed with exit code 1.

@noencke
Copy link
Contributor

noencke commented Mar 6, 2025

This seems like a useful tool in certain scenarios. Have you considered an alternative API, where, rather than there being a method on SchemaFactory, you may instead pass in a pre-existing factory when constructing a new factory?

const sfA = new SchemaFactory("A");
const sfB = new SchemaFactory({ parentScope: sfA, scope: "B" });
class C extends sfB.object("C", {}) {}
// C.identifier === "A.B.C"

This is slightly less convenient to invoke, but it also keeps the feature (nested scoping) isolated to precisely where it matters (when defining new scopes) rather than having it be something that pops up in autocomplete every time you type sf.. @Josmithr , curious for your thoughts as well when you have time.

Also, is there a reason this is being introduced on SchemaFactory and not SchemaFactoryAlpha?

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 area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct base: main PRs targeted against main branch changeset-present public api change Changes to a public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants