Skip to content

Include final retain in tree deltas - #27809

Open
CraigMacomber wants to merge 4 commits into
microsoft:mainfrom
CraigMacomber:quillLastLine
Open

Include final retain in tree deltas#27809
CraigMacomber wants to merge 4 commits into
microsoft:mainfrom
CraigMacomber:quillLastLine

Conversation

@CraigMacomber

Copy link
Copy Markdown
Contributor

Description

Previously our array node deltas were not required to include the final retain. This is now alwayse included.

Addationally handling of the extra new line required to make quill a happe is now more robust, fixing a buig where remote copllaborators should end up with extra new lines.

Reviewer Guidance

The review process is outlined on this wiki page.

Copilot AI review requested due to automatic review settings July 31, 2026 18:43
@CraigMacomber
CraigMacomber requested review from a team as code owners July 31, 2026 18:43
@github-actions github-actions Bot added base: main PRs targeted against main branch area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct area: tools area: dds Issues related to distributed data structures area: repo Repo related work area: website area: dds: tree changeset-present labels Jul 31, 2026
@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Hi! Thank you for opening this PR. Want me to review it?

Based on the diff (488 lines, 12 files), I've queued these reviewers:

  • Correctness — logic errors, race conditions, lifecycle issues
  • Security — vulnerabilities, secret exposure, injection
  • API Compatibility — breaking changes, release tags, type design
  • Performance — algorithmic regressions, memory leaks
  • Testing — coverage gaps, hollow tests

How this works

  • Adjust the reviewer set by ticking/unticking boxes above. Reviewer toggles alone don't trigger anything.

  • Tick Start review below to dispatch the review fleet.

  • After review finishes, tick Start review again to request another run — it auto-resets after each dispatch.

  • This comment updates as new commits land; your reviewer selections are preserved.

  • Start review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

@Josmithr
Josmithr requested a review from daesunp July 31, 2026 19:33
Comment thread packages/framework/quill-react/src/formatted/quillFormattedView.tsx
Comment thread packages/framework/quill-react/src/formatted/quillFormattedView.tsx
Comment thread packages/dds/tree/src/simple-tree/api/treeNodeApi.ts
Comment thread packages/framework/quill-react/src/test/textEditor.test.tsx
Comment thread packages/framework/quill-react/src/test/textEditor.test.tsx
CraigMacomber added a commit that referenced this pull request Jul 31, 2026
## Description

While authoring #27809 I
had to make some assumptions about deltas, which were undocumented.

This fills in the missing documentation.
@dannimad

dannimad commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

wait for #27824 to merge

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Bundle size comparison

Base commit: f3785dfc769ab502e10e1123733c60835e7ad73c
Head commit: a61bb2e53ad074c277050eeb961e71d1bffea991

Notable changes

No bundles changed by ≥ 500 bytes parsed.

Per-bundle deltas

@fluid-example/bundle-size-tests

  • azureClient.js: parsed 620351 → 620407 (+56), gzip 165480 → 165527 (+47)
  • odspClient.js: parsed 592639 → 592695 (+56), gzip 158606 → 158653 (+47)
  • aqueduct.js: parsed 526970 → 527005 (+35), gzip 141035 → 141063 (+28)
  • fluidFramework.js: parsed 398036 → 398254 (+218), gzip 112955 → 113065 (+110)
  • sharedTree.js: parsed 387440 → 387651 (+211), gzip 110380 → 110486 (+106)
  • containerRuntime.js: parsed 305091 → 305105 (+14), gzip 83553 → 83558 (+5)
  • sharedString.js: parsed 176457 → 176464 (+7), gzip 49807 → 49815 (+8)
  • experimentalSharedTree.js: parsed 160677 → 160677 (0), gzip 46276 → 46276 (0)
  • matrix.js: parsed 160302 → 160309 (+7), gzip 45800 → 45807 (+7)
  • loader.js: parsed 145459 → 145473 (+14), gzip 39221 → 39235 (+14)
  • odspDriver.js: parsed 103906 → 103927 (+21), gzip 32404 → 32411 (+7)
  • directory.js: parsed 67110 → 67117 (+7), gzip 18859 → 18866 (+7)
  • 578.js: parsed 58686 → 58686 (0), gzip 17657 → 17657 (0)
  • map.js: parsed 47205 → 47212 (+7), gzip 14455 → 14462 (+7)
  • odspPrefetchSnapshot.js: parsed 45635 → 45649 (+14), gzip 15242 → 15249 (+7)
  • 252.js: parsed 44371 → 44371 (0), gzip 13732 → 13732 (0)
  • summarizerDelayLoadedModule.js: parsed 30717 → 30717 (0), gzip 7716 → 7716 (0)
  • socketModule.js: parsed 26469 → 26476 (+7), gzip 7895 → 7903 (+8)
  • createNewModule.js: parsed 12454 → 12454 (0), gzip 4797 → 4797 (0)
  • summaryModule.js: parsed 3789 → 3789 (0), gzip 1857 → 1857 (0)
  • connectionState.js: parsed 909 → 909 (0), gzip 500 → 500 (0)
  • sharedTreeAttributes.js: parsed 847 → 854 (+7), gzip 499 → 508 (+9)
  • debugAssert.js: parsed 429 → 429 (0), gzip 299 → 299 (0)
  • FluidFramework-HashFallback.js: parsed 419 → 419 (0), gzip 313 → 313 (0)

}

// Fixup required new line: quill requires one at the end, so we add an extra if needed.
// This simple deletes the old one if there was one, then adds a new one if needed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
// This simple deletes the old one if there was one, then adds a new one if needed.
// This simply deletes the old one if there was one, then adds a new one if needed.

Comment on lines +332 to +333
// This hits an edge case in the quill integration since quill requires such a trailing line end,
// so our integration replaces its extra one it had to add to support will with the real one added by this change.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There are some typos here.

@Josmithr Josmithr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Left a couple of comments for docs typos. Otherwise, looks good to me.

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 area: repo Repo related work area: tools area: website base: main PRs targeted against main branch changeset-present

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants