Skip to content

refactor(tree): Reserve positional argument in treeChanged event listener signature - #27951

Open
Joshua Smithrud (Josmithr) wants to merge 6 commits into
microsoft:mainfrom
Josmithr:tree/reserve-treeChanged-arg
Open

refactor(tree): Reserve positional argument in treeChanged event listener signature#27951
Joshua Smithrud (Josmithr) wants to merge 6 commits into
microsoft:mainfrom
Josmithr:tree/reserve-treeChanged-arg

Conversation

@Josmithr

@Josmithr Joshua Smithrud (Josmithr) commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

TreeAlpha's treeChanged events already propogate delta events through this surface at runtime, so this is strictly formalizing an expectation that must already be satisfied to in order to safely use these events.

@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 public api change Changes to a public API area: dds: tree changeset-present labels Aug 13, 2026
@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

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

Based on the diff (59 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.

Pull request overview

This PR updates the stable Simple Tree treeChanged event listener signature to reserve a positional argument (unstable?: unknown) for potential future/experimental event payloads, aligning it with the existing approach used by nodeChanged.

Changes:

  • Updates TreeChangeEvents.treeChanged to treeChanged(unstable?: unknown): void and documents the reserved-argument guidance.
  • Regenerates API reports across @fluidframework/tree and the fluid-framework umbrella package to reflect the new signature.
  • Adds a changeset announcing the minor-version API surface change and providing migration guidance for listeners.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/dds/tree/src/simple-tree/api/treeChangeEvents.ts Updates treeChanged signature and adds reserved-argument documentation (plus minor doc cleanup).
packages/dds/tree/api-report/tree.public.api.md API report update for TreeChangeEvents.treeChanged signature.
packages/dds/tree/api-report/tree.legacy.public.api.md Legacy API report update for TreeChangeEvents.treeChanged signature.
packages/dds/tree/api-report/tree.legacy.beta.api.md Legacy beta API report update for TreeChangeEvents.treeChanged signature.
packages/dds/tree/api-report/tree.beta.api.md Beta API report update for TreeChangeEvents.treeChanged signature.
packages/dds/tree/api-report/tree.alpha.api.md Alpha API report update for TreeChangeEvents.treeChanged signature.
packages/framework/fluid-framework/api-report/fluid-framework.public.api.md Umbrella package API report update mirroring the TreeChangeEvents.treeChanged signature change.
packages/framework/fluid-framework/api-report/fluid-framework.legacy.public.api.md Umbrella legacy API report update mirroring the signature change.
packages/framework/fluid-framework/api-report/fluid-framework.legacy.beta.api.md Umbrella legacy beta API report update mirroring the signature change.
packages/framework/fluid-framework/api-report/fluid-framework.beta.api.md Umbrella beta API report update mirroring the signature change.
packages/framework/fluid-framework/api-report/fluid-framework.alpha.api.md Umbrella alpha API report update mirroring the signature change.
.changeset/reserve-tree-changed-argument.md Changeset documenting the reserved argument, runtime behavior, and safe listener patterns.
Suppressed comments (1)

packages/dds/tree/src/simple-tree/api/treeChangeEvents.ts:103

  • Same grammar issue as above ("added from breaking"). Rephrase so the guidance reads clearly and avoids ungrammatical wording.
	 * Using an inline lambda expression as the listener callback is a good pattern to avoid cases like this where
	 * arguments are added from breaking due to optional arguments.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/dds/tree/src/simple-tree/api/treeChangeEvents.ts
The event's runtime behavior has not changed, but the declaration reserves that position for event data that experimental or future APIs may provide.

Most listeners require no changes.
Listeners that declare their own optional first parameter should remove it or use a wrapper so they do not interpret event data as application data.

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.

Might be worth adjusting the phrasing in this changeset to better communicate that this is not changing runtime behavior, and the runtime behavior which such code would mishandle is already happening.

@github-actions

Copy link
Copy Markdown
Contributor

🔗 Found some broken links! 💔

Run a link check locally to find them. See Checking for Broken Links for more information.

linkcheck output

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"


> fluid-framework-website@0.0.0 serve
> docusaurus serve --no-open

[SUCCESS] Serving "build" directory at: http://localhost:3000/
[ELIFECYCLE] Command failed with exit code 1.

@github-actions

Copy link
Copy Markdown
Contributor

Bundle size comparison

Base commit: 56642dd4bc150ae8f38c99e0eaa134db087e2211
Head commit: 849512d8439ce6945d46f74aaef1a9fc414ef4f6

Notable changes

No bundles changed by ≥ 500 bytes parsed.

Per-bundle deltas

@fluid-example/bundle-size-tests

  • fluidFrameworkAllAlpha.js: parsed 784199 → 784241 (+42), gzip 214950 → 214985 (+35)
  • azureClient.js: parsed 624847 → 624903 (+56), gzip 166641 → 166688 (+47)
  • odspClient.js: parsed 597135 → 597191 (+56), gzip 159785 → 159828 (+43)
  • aqueduct.js: parsed 531223 → 531258 (+35), gzip 142114 → 142144 (+30)
  • fluidFramework.js: parsed 403499 → 403520 (+21), gzip 114397 → 114414 (+17)
  • sharedTree.js: parsed 392903 → 392917 (+14), gzip 111836 → 111845 (+9)
  • containerRuntime.js: parsed 309144 → 309158 (+14), gzip 84569 → 84576 (+7)
  • sharedString.js: parsed 176510 → 176517 (+7), gzip 49798 → 49806 (+8)
  • experimentalSharedTree.js: parsed 160665 → 160665 (0), gzip 46265 → 46265 (0)
  • matrix.js: parsed 160341 → 160348 (+7), gzip 45798 → 45805 (+7)
  • loader.js: parsed 145704 → 145718 (+14), gzip 39286 → 39302 (+16)
  • 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 → 15250 (+8)
  • 252.js: parsed 44362 → 44362 (0), gzip 13735 → 13735 (0)
  • summarizerDelayLoadedModule.js: parsed 30717 → 30717 (0), gzip 7716 → 7716 (0)
  • socketModule.js: parsed 26469 → 26476 (+7), gzip 7896 → 7904 (+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)

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 public api change Changes to a public API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants