feat(tree): history and rewind - #28012
Conversation
|
Hi! Thank you for opening this PR. Want me to review it? Based on the diff (1170 lines, 22 files), I've queued these reviewers:
How this works
|
| --- | ||
| New alpha APIs for inspecting history and restoring past states | ||
|
|
||
| `UntypedTreeViewAlpha` now exposes a `branchHistory` property which returns a `TreeBranchHistory` object with: |
There was a problem hiding this comment.
Nit: can we link to API docs for at least existing APIs here?
There was a problem hiding this comment.
UntypedTreeViewAlpha is not in the docs yet, so I can't link to it.
| * The original underlying branch will be disposed unless it is the main branch or a {@link (ITreeAlpha:interface).createSharedBranch | shared branch}. | ||
| * In order to retain the local branch, consider {@link UntypedTreeViewAlpha.fork | forking} before rewinding. | ||
| */ | ||
| rewindTo(revision: string): void; |
There was a problem hiding this comment.
Nit: Is it safe to assume that the format of the revision is intended to be opaque to users? I'm wondering if there might be any value in creating a type alias for them that documents any expectations in a single place.
Joshua Smithrud (Josmithr)
left a comment
There was a problem hiding this comment.
Left a couple minor suggestions, but otherwise docs and API changes look good. I didn't look at the implementation details in much detail though, would be good to get another tree approval there.
| * The original underlying branch will be disposed unless it is the main branch or a {@link (ITreeAlpha:interface).createSharedBranch | shared branch}. | ||
| * In order to retain the local branch, consider {@link UntypedTreeViewAlpha.fork | forking} before rewinding. | ||
| */ | ||
| rewindTo(revision: string): void; |
There was a problem hiding this comment.
I think revertTo is a good name and aligns with Revertible.
Is "rewind" too similar? Will customers get confused? I can't think of a better name at the moment though... maybe we just refine the API later
Bundle size comparisonBase commit: Pending — |
|
🔗 No broken links found! ✅ Your attention to detail is admirable. linkcheck output |
…17 (#28104) ## Description Cherry-picks the SharedTree history and persisted-commit-metadata work onto the `release/client/2.117` branch. This is step 4 of the 2.117 release plan (branch created from `release/client/2.116` + version bump); release notes, changelogs and assert tagging are handled separately by the release engineers. Four commits, applied in merge order (each with `git cherry-pick -x`, so the original SHA is recorded in the commit message): | Commit on `main` | PR | What it provides | |---|---|---| | `22e5b4ee` | [#27932](#27932) | Renames the alpha `TreeBranchAlpha` interface to `UntypedTreeViewAlpha` (old name kept as a deprecated alias, so it is additive). Carries no feature, but #28012 is written against the new name, so it is a prerequisite rather than an optional cleanup. | | `3b665a2a` | [#28036](#28036) | `retainHistory` now retains the trunk in *summaries*, not just in memory. Without it, retained history is discarded at the next summary and never reaches clients that load from it. No persisted format change; the `retainHistory: false` default is untouched. | | `920e9469` | [#28012](#28012) | `UntypedTreeViewAlpha.branchHistory` (`length`, `getHead()` → `TreeBranchCommitMetadata { revision, getParent() }`), plus `rewindTo(revision)` and `revertTo(revision, options?)`. | | `90337165` | [#28064](#28064) | Persisted commit metadata: `customMetadata` on `RunTransactionParamsAlpha` and the revert options, read back as `.custom` / `.customTree`. Stored inline on commits in the `EditManager` summary under `EditManagerFormatVersion.v7` / `MessageFormatVersion.v7`, written only when `minVersionForCollab` is at least `2.117.0`. | A fifth commit adapts the picks to this branch — details below. ### Why a minor rather than a patch #28064 changes a persisted format, and the format gate is a version string baked into SharedTree's source. The config map that selects a format version rejects patch versions, so the gate has to be an `X.Y.0`. It is `FluidClientVersion.v2_117 = "2.117.0"`. ### Forward compatibility with 3.0 All four commits are already on `main`, and `main`'s tip was #28064 when this was prepared, so the two lines could be compared directly. Everything that ships here is identical to 3.0: - The whole persisted-format surface — `src/codec/` and `src/shared-tree-core/` — is byte-identical between this branch and `main`, including the `v2_117` → v7 gate. A mixed 2.117/3.0 session cannot silently disagree about the format. - Every alpha symbol added here (`TreeBranchHistory`, `TreeBranchCommitMetadata`, `branchHistory`, `rewindTo`, `revertTo`, `customMetadata`, `customTree`, `retainHistory`, `UntypedTreeViewAlpha`, `RunTransactionParamsAlpha`, `RevertOptionsAlpha`, `RevertToOptionsAlpha`) is identical on both lines, at the same API tier, with the same deprecation state. - `minVersionForCollab: '2.117.0'` stays valid on 3.0: `OldestSupportedClientVersion` *widens* from `` `${1 | 2}.${bigint}.${bigint}` `` to `` `${1 | 2 | 3}....` ``, so consumers who adopt 2.117 need no change in this area when they later move to 3.0. ## Adapting the picks to this branch `main` is on TypeScript 6 and past the 3.0 bump; this branch is TypeScript 5.4 and pre-3.0. Conflicts were all of one shape — an import list where `main` has accumulated names from commits that are *not* being backported — and were resolved by keeping only what this branch actually uses: - `src/index.ts` — kept `asTreeViewAlpha` (removed on `main` by 3.0 work) alongside the incoming `TreeBranchCommitMetadata` / `TreeBranchHistory`. - `src/shared-tree/treeCheckout.ts` — kept `StableId` and `findAncestor`, which #28012 uses; dropped `tagCodeArtifacts` (used on `main` only by schema-change telemetry, #27996) and `getDeltaChangeProfile` (introduced by #27989). Neither is backported here. - `src/test/shared-tree/schematizeTree.spec.ts` — kept `TreeBranchHistory`; dropped `UntypedTreeViewAlpha`, which was added to that import by the TypeScript 6 upgrade (#28052) rather than by #28012. One genuine TypeScript-version difference, in the fifth commit: - `customCommitMetadata.spec.ts` used `.filter((m) => m !== undefined)` and then read `.tag`, relying on **inferred type predicates**, a TypeScript 5.5 feature. On 5.4 the element type stays `T | undefined` and the compiler reports TS18048. The predicate is now spelled explicitly. This was the only TypeScript error in the entire build. The two conflicted `*.api.md` files are generated artifacts, so rather than hand-resolving them they were regenerated by a full clean build from the repo root. That correctly drops `asAlpha`, `codePointCount` and `utf16LengthForCodePoints` — whose exports come from #28011 and #28004, not backported here — and narrows `OldestSupportedServiceClientVersion` to `` `2.${bigint}.0` ``. ## Validation - Full `pnpm clean && pnpm build` from the repo root: succeeded, and left the working tree clean apart from the intended API report updates. No unexpected API report drift. - `@fluidframework/tree` test suite: **15301 passing, 488 pending, 1 failing**. The one failure is a pre-existing Windows-only issue in `snapshotCompatibilityChecker.spec.ts`, which builds the directory under test with `path.join(...)` but hardcodes forward slashes in the expected error string. It is untouched by these commits and passes on Linux CI. ## Reviewer Guidance The review process is outlined in [the pull request guidelines](../docs/content/Contributing/PR-Guidelines.md#guidelines). - The first four commits are unmodified cherry-picks; review effort is best spent on the fifth (`fix(tree): adapt the backported history work to the 2.117 line`) and on the conflict resolutions described above, since those are the only places this branch diverges from what was reviewed on `main`. - **Assert tagging is deliberately not included here.** The three new asserts are still untagged string literals, so `flub release prepare client` will report them. Running `flub generate assertTags` on a release branch allocates short codes from the branch's own high-water mark, which can disagree with `main` — on `main` today it would reassign `0xd33`, a code that shipped in 2.116 as `"compatibilityMode must be defined"`, to a tree assert. Happy to follow whatever sequencing the release engineers prefer. --------- Co-authored-by: jzaffiro <110866475+jzaffiro@users.noreply.github.com> Co-authored-by: yann-achard-MS <97201204+yann-achard-MS@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3af96e03-702e-49da-adaa-bc99ca75ee27 Copilot-Session: eedf6254-a76e-4efe-af90-1f7b43da8665 Copilot-Session: a037b96b-478f-4a65-9555-d7a970e7855e Copilot-Session: 42b443d7-0621-42a1-b087-f4e4765046af Copilot-Session: 797fba3e-0e9a-48db-86c6-530aa2837434
This reverts commit 920e946.
This reverts commit 920e946.
Description
Adds features to allow applications to rewind a view to a specific revision by switching to a new SharedTreeBranch that is forked from that revision.
Breaking Changes
None