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

fix(tree): insert* and move* should accept proxy API types #17972

Merged
merged 2 commits into from
Oct 25, 2023

Conversation

DLehenbauer
Copy link
Contributor

SharedListNode.insert*() needed adjustment to accept the new factory-created content.
SharedListNode.move*() needed adjustment to accept to accept a SharedListNode as source.

Testing also uncovered and fixed a couple of minor bugs in the move implementation:

  • Use 'boxedAt()' when validating that the items in the source list comply with the destination type.
  • The adjustment of the destination index wasn't quite right.

@DLehenbauer DLehenbauer requested review from a team as code owners October 25, 2023 13:57
@DLehenbauer DLehenbauer requested review from noencke and removed request for a team October 25, 2023 13:57
@github-actions github-actions bot added area: dds Issues related to distributed data structures area: dds: tree public api change Changes to a public API base: main PRs targeted against main branch labels Oct 25, 2023
@msfluid-bot
Copy link
Collaborator

@fluid-example/bundle-size-tests: +311 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 445.96 KB 445.96 KB No change
connectionState.js 680 Bytes 680 Bytes No change
containerRuntime.js 239.05 KB 239.05 KB No change
loader.js 178.73 KB 178.73 KB No change
map.js 48.06 KB 48.06 KB No change
matrix.js 141.84 KB 141.84 KB No change
odspDriver.js 90.27 KB 90.27 KB No change
odspPrefetchSnapshot.js 41.9 KB 41.9 KB No change
sharedString.js 162.75 KB 162.75 KB No change
sharedTree2.js 262.91 KB 263.21 KB +311 Bytes
Total Size 1.72 MB 1.72 MB +311 Bytes

Baseline commit: 0d1436b

Generated by 🚫 dangerJS against 541b568

Copy link
Contributor

@noencke noencke left a comment

Choose a reason for hiding this comment

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

Awesome!

if (destinationIndex > sourceStart) {
destinationIndex =
destinationIndex < sourceEnd
? sourceStart // distination overlaps with source range -> slide to left
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
? sourceStart // distination overlaps with source range -> slide to left
? sourceStart // destination overlaps with source range -> slide to left

This typo made me realize that while "src" and "dest" are opposites, so are "src" and "dist".

@DLehenbauer DLehenbauer merged commit a687b7f into microsoft:main Oct 25, 2023
28 checks passed
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 public api change Changes to a public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants