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.

Sign up for GitHub

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

Jump to bottom

tree: Const schema type parameters, more tests tree and misc cleanups #22927

Merged
merged 2 commits into from
Oct 29, 2024
Merged

tree: Const schema type parameters, more tests tree and misc cleanups #22927

merged 2 commits into from
Oct 29, 2024

Conversation

CraigMacomber
Copy link
Contributor

Description

A collection of minor improvements split out from #22566 for simplified and more targeted review.

Reviewer Guidance

The review process is outlined on this wiki page.

@CraigMacomber CraigMacomber requested review from a team as code owners October 29, 2024 20:51
@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 public api change Changes to a public API base: main PRs targeted against main branch labels Oct 29, 2024
CraigMacomber
CraigMacomber commented Oct 29, 2024
View reviewed changes
packages/dds/tree/api-report/tree.beta.api.md
@@ -449,7 +449,7 @@ export interface TreeArrayNodeUnsafe<TAllowedTypes extends Unenforced<ImplicitAl
// @beta @sealed
export const TreeBeta: {
on<K extends keyof TreeChangeEventsBeta<TNode>, TNode extends TreeNode>(node: TNode, eventName: K, listener: NoInfer<TreeChangeEventsBeta<TNode>[K]>): () => void;
clone<TSchema extends ImplicitFieldSchema>(node: TreeFieldFromImplicitField<TSchema>): TreeFieldFromImplicitField<TSchema>;
clone<const TSchema extends ImplicitFieldSchema>(node: TreeFieldFromImplicitField<TSchema>): TreeFieldFromImplicitField<TSchema>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When combined with the recent corrections to schema variance, this change improves handling of passing array literals to this function as the schema for a union, since it removes the need to specify "as const" at the call site.

CraigMacomber
CraigMacomber commented Oct 29, 2024
View reviewed changes
packages/dds/tree/src/simple-tree/api/conciseTree.ts
@@ -16,13 +16,14 @@ import { getUnhydratedContext } from "../createContext.js";
* @remarks
* This is "concise" meaning that explicit type information is omitted.
* If the schema is compatible with {@link ITreeConfigurationOptions.preventAmbiguity},
* types will be lossless and compatible with {@link TreeBeta.create} (unless the options are used to customize it).
* types will be lossless and compatible with {@link TreeAlpha.create} (unless the options are used to customize it).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

neither of these links works yet, but this is the API thats getting added in #22566

@github-actions GitHub Actions
Copy link
Contributor

🔗 No broken links found! ✅

Your attention to detail is admirable.

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...

Stats:
  436034 links
    3364 destination URLs
       2 URLs ignored
       0 warnings
       0 errors

@CraigMacomber CraigMacomber enabled auto-merge (squash) October 29, 2024 21:09
msfluid-bot
msfluid-bot reviewed Oct 29, 2024
View reviewed changes
Copy link
Collaborator

@msfluid-bot msfluid-bot left a comment

Choose a reason for hiding this comment

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

Code Coverage Summary

↑ packages.dds.tree.src.simple-tree.api:
Line Coverage Change: 0.05%    Branch Coverage Change: 0.04%
Metric NameBaseline coveragePR coverageCoverage Diff
Branch Coverage 86.49% 86.53% ↑ 0.04%
Line Coverage 81.68% 81.73% ↑ 0.05%

Baseline commit: 645b9ed
Baseline build: 303023
Happy Coding!!

Code coverage comparison check passed!!

@msfluid-bot
Copy link
Collaborator

⯅ @fluid-example/bundle-size-tests: +245 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 464.08 KB 464.11 KB ⯅ +35 Bytes
azureClient.js 561.65 KB 561.7 KB ⯅ +49 Bytes
connectionState.js 724 Bytes 724 Bytes ■ No change
containerRuntime.js 261.67 KB 261.68 KB ⯅ +14 Bytes
fluidFramework.js 406.28 KB 406.29 KB ⯅ +14 Bytes
loader.js 134.16 KB 134.18 KB ⯅ +14 Bytes
map.js 42.71 KB 42.71 KB ⯅ +7 Bytes
matrix.js 148.54 KB 148.54 KB ⯅ +7 Bytes
odspClient.js 528.18 KB 528.23 KB ⯅ +49 Bytes
odspDriver.js 97.84 KB 97.86 KB ⯅ +21 Bytes
odspPrefetchSnapshot.js 42.81 KB 42.82 KB ⯅ +14 Bytes
sharedString.js 164.73 KB 164.74 KB ⯅ +7 Bytes
sharedTree.js 396.74 KB 396.74 KB ⯅ +7 Bytes
Total Size 3.32 MB 3.32 MB ⯅ +245 Bytes

Baseline commit: 645b9ed

Generated by 🚫 dangerJS against c4840de

@CraigMacomber CraigMacomber merged commit 4a730a0 into microsoft:main Oct 29, 2024
32 checks passed
@CraigMacomber CraigMacomber deleted the constSchemes branch October 29, 2024 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msfluid-bot msfluid-bot msfluid-bot left review comments

@Josmithr Josmithr Josmithr approved these changes

Assignees
No one assigned
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 public api change Changes to a public API
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@CraigMacomber @msfluid-bot @Josmithr