refactor(tree2): tree builds can be centralized in ModularChangeFamily changesets #18386
Conversation
github-actions
bot
added
base: main
⯅ @fluid-example/bundle-size-tests: +871 Bytes
Baseline commit: 0bb85e9 |
| @@ -121,7 +122,7 @@ function invertMark<TNodeChange>( | |||
| id: mark.id, | |||
| cellId: outputId, | |||
| count: mark.count, | |||
| detachIdOverride: mark.cellId, | |||
| detachIdOverride: getInputCellId(mark, revision, revisionMetadata), | |||
There was a problem hiding this comment.
Is this related to the rest of the PR?
There was a problem hiding this comment.
It was needed when I was trying to avoid using AttachAndDetach marks for cases where the attach is a new insert.
As we dicussed I can't make that work yet because we need to keep the insert effect (so we can compare its ID to the the ID of the cell).
That said, this change is still good. I've just pushed a pair of tests that fail if getInputCellId is not used.
| return undefined; | ||
| } | ||
| const encoded: EncodedBuilds = nestedMapToFlatList(builds).map(([r, i, t]) => | ||
| r !== undefined ? [r, i, t] : [i, t], |
There was a problem hiding this comment.
This feels like a potentially unnecessary optimization. Maybe at least add a quick comment either here or in the format file about this optimization.
There was a problem hiding this comment.
Why unnecessary? Here's the comment I'm considering:
// `undefined` does not round-trip through JSON strings, so it needs special handling.
// Most entries will have an undefined revision due to the revision information being inherited from the `ModularChangeset`.
// We therefore optimize for the common case by omitting the revision when it is undefined.
Key changes:
Note that in this first version, the builds are atomized.
Punted: