docs: More scope docs #23026

CraigMacomber · 2024-11-07T22:09:30Z

Description

More docs for SchemaFactory.scope.

Convert docs to other format for parameter properties so it can have its own subsections.

Reviewer Guidance

The review process is outlined on this wiki page.

packages/framework/fluid-framework/api-report/fluid-framework.legacy.public.api.md

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

msfluid-bot

Code Coverage Summary

↑ packages.dds.tree.src.simple-tree.api:
Line Coverage Change: 0.09% Branch Coverage Change: 0.03%

Metric Name	Baseline coverage	PR coverage	Coverage Diff
Branch Coverage	88.73%	88.76%	↑ 0.03%
Line Coverage	82.19%	82.28%	↑ 0.09%

Baseline commit: d1ba184
Baseline build: 305646
Happy Coding!!

Code coverage comparison check passed!!

vladsud · 2024-11-07T22:56:44Z

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

@@ -223,12 +223,36 @@ export class SchemaFactory<
 	 * If each library exporting schema picks its own globally unique scope for its SchemaFactory,
 	 * then all schema an application might depend on, directly or transitively,
 	 * will end up with a unique fully qualified name which is required to refer to it in persisted data and errors.


I can't parse that block. What does this refer to? "require to refer" - what is the API / workflow that it refers to?

I have rewritten this to be simpler so it's less of a toss up if you happen to parse the sentence in a way that works.

msfluid-bot · 2024-11-07T22:57:25Z

⯅ @fluid-example/bundle-size-tests: +245 Bytes

Metric Name	Baseline Size	Compare Size	Size Diff
aqueduct.js	463.74 KB	463.77 KB	⯅ +35 Bytes
azureClient.js	562.69 KB	562.74 KB	⯅ +49 Bytes
connectionState.js	724 Bytes	724 Bytes	■ No change
containerRuntime.js	261.99 KB	262 KB	⯅ +14 Bytes
fluidFramework.js	426.65 KB	426.66 KB	⯅ +14 Bytes
loader.js	134.18 KB	134.19 KB	⯅ +14 Bytes
map.js	42.71 KB	42.71 KB	⯅ +7 Bytes
matrix.js	148.32 KB	148.33 KB	⯅ +7 Bytes
odspClient.js	528.43 KB	528.48 KB	⯅ +49 Bytes
odspDriver.js	97.84 KB	97.86 KB	⯅ +21 Bytes
odspPrefetchSnapshot.js	42.81 KB	42.83 KB	⯅ +14 Bytes
sharedString.js	164.17 KB	164.17 KB	⯅ +7 Bytes
sharedTree.js	417.11 KB	417.11 KB	⯅ +7 Bytes
Total Size	3.36 MB	3.36 MB	⯅ +245 Bytes

Baseline commit: d1ba184

Generated by 🚫 dangerJS against 03031e9

vladsud · 2024-11-07T23:06:05Z

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

+		 * @remarks
+		 * Generally each library which is developed independently (possible a package, but could also be part of a package or multiple packages developed together) should get its own globally unique `scope`.
+		 * This ensures that the given library can name its schema (via the name parameter passed to the schema building methods, or implicitly for structurally named schema)
+		 * in a way that must only be unique within the library.


I can't parse "library can name in a way that must be unique" part. The mix of "can" and "must" is weird to me (but maybe I do not get details of beautiful English language :))

I think you want to say that if two distinct types happen to have same runtime ID (which is schema factory ID + type ID) then really bad things would happen (maybe describe what happens and point to "help with debugging" article). In order for types to have unique runtime IDs, they need to ensure combination of factory schema ID + type ID is unique in a container.

Only after that intro, I'd give advice on usage of revers domain names - it's somewhat misplaced IMHO in the sequence.

I have simplified the grammar and reordered the docs accordingly. Should be better now.

Co-authored-by: Joshua Smithrud <[email protected]>

vladsud · 2024-11-08T23:00:48Z

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

+		 * Prefix appended to the identifiers of all {@link TreeNodeSchema} produced by this builder.
+		 *
+		 * @remarks
+		 * Generally each developed independently library (possibly a package, but could also be part of a package or multiple packages developed together) should get its own globally unique `scope`.


I'd probably change order slightly.

I'd start with "These are joined ..." sentence.
Then say - resulting schema identifier uniquely defines type in Tree instance ("globally" is not right, as not clear what is the scope of "globe"). And if two different types (having different layout) get same schema identifier, then really bad things happen (might be worth pointing to what exactly happens - might be useful for OCEs).

Only then I'd move to guidance on how to choose SchemaFactory.scope.

Micrisoft (and some other users of GUIDs) likes to use "Global" to describe the scope in which identifiers which should never collide with anything else, for example https://learn.microsoft.com/en-us/previous-versions/windows/desktop/automat/globally-unique-identifier-guid-

I have rephrased the docs to avoid referring to global as it might not be obvious to people no subjected to using GUIDs all the time. I'm glad to know everyone is familiar with the term.

Also the identifier isn't for identifying types in a tree instance: we use the schema object identity for that. Instead its for associating schema with persisted data. The linked documentation on TreeNodeSchemaCore,identifier covers this, so I don't think adding details about it inline here is necessary.

Microsoft is just being honest - we promise to empower everyone on the planet to achieve more, but not everyone in the universe.

github-actions · 2024-11-09T02:18:33Z

🔗 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:linkcheck /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test ci:start 1313 linkcheck:full

1: starting server using command "npm run ci:start"
and when url "[ 'http://127.0.0.1:1313' ]" is responding with HTTP status code 200
running tests using command "npm run linkcheck:full"


> [email protected] ci:start
> http-server ./public --port 1313 --silent


> [email protected] linkcheck:full
> npm run linkcheck:fast -- --external


> [email protected] linkcheck:fast
> linkcheck http://localhost:1313 --skip-file skipped-urls.txt --external

Crawling...

http://localhost:1313/docs/start/tree-start/
- (3430:89) 'here' => https://github.com/microsoft/FluidFramework/blob/main/packages/dds/tree/docs/main/merge-semantics.md (HTTP 404)


Stats:
  443666 links
    3413 destination URLs
       2 URLs ignored
       0 warnings
       1 errors

 ELIFECYCLE  Command failed with exit code 1.

More scope docs

86b6014

CraigMacomber requested review from a team as code owners November 7, 2024 22:09

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 public api change Changes to a public API base: main PRs targeted against main branch labels Nov 7, 2024

Abe27342 approved these changes Nov 7, 2024

View reviewed changes

Josmithr reviewed Nov 7, 2024

View reviewed changes

packages/framework/fluid-framework/api-report/fluid-framework.legacy.public.api.md Show resolved Hide resolved

Josmithr reviewed Nov 7, 2024

View reviewed changes

packages/dds/tree/src/simple-tree/api/schemaFactory.ts Outdated Show resolved Hide resolved

Josmithr reviewed Nov 7, 2024

View reviewed changes

packages/dds/tree/src/simple-tree/api/schemaFactory.ts Outdated Show resolved Hide resolved

msfluid-bot reviewed Nov 7, 2024

View reviewed changes

vladsud reviewed Nov 7, 2024

View reviewed changes

CraigMacomber and others added 3 commits November 7, 2024 16:32

Apply suggestions from code review

3ba45b5

Co-authored-by: Joshua Smithrud <[email protected]>

Simplify sentences

0a2987d

Reorder docs

b02ed69

vladsud reviewed Nov 8, 2024

View reviewed changes

vladsud approved these changes Nov 8, 2024

View reviewed changes

Editing

03031e9

CraigMacomber enabled auto-merge (squash) November 9, 2024 02:16

CraigMacomber merged commit 5405422 into microsoft:main Nov 9, 2024
30 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: More scope docs #23026

docs: More scope docs #23026

CraigMacomber commented Nov 7, 2024 •

edited by tylerbutler

Loading

msfluid-bot left a comment •

edited

Loading

vladsud Nov 7, 2024

CraigMacomber Nov 8, 2024

msfluid-bot commented Nov 7, 2024 •

edited

Loading

vladsud Nov 7, 2024

CraigMacomber Nov 8, 2024

vladsud Nov 8, 2024

CraigMacomber Nov 9, 2024

noencke Nov 11, 2024

github-actions bot commented Nov 9, 2024

docs: More scope docs #23026

docs: More scope docs #23026

Conversation

CraigMacomber commented Nov 7, 2024 • edited by tylerbutler Loading

Description

Reviewer Guidance

msfluid-bot left a comment • edited Loading

Choose a reason for hiding this comment

Code Coverage Summary

Code coverage comparison check passed!!

vladsud Nov 7, 2024

Choose a reason for hiding this comment

CraigMacomber Nov 8, 2024

Choose a reason for hiding this comment

msfluid-bot commented Nov 7, 2024 • edited Loading

vladsud Nov 7, 2024

Choose a reason for hiding this comment

CraigMacomber Nov 8, 2024

Choose a reason for hiding this comment

vladsud Nov 8, 2024

Choose a reason for hiding this comment

CraigMacomber Nov 9, 2024

Choose a reason for hiding this comment

noencke Nov 11, 2024

Choose a reason for hiding this comment

github-actions bot commented Nov 9, 2024

linkcheck output

CraigMacomber commented Nov 7, 2024 •

edited by tylerbutler

Loading

msfluid-bot left a comment •

edited

Loading

msfluid-bot commented Nov 7, 2024 •

edited

Loading