Fix bug in SharedTree factory proxy hydration

[noencke]

Description

There currently exists a bug during proxy hydration. In the proxy API, proxies in the input tree are immediately hydrated after inserting content into the tree, however, tree-change events are nonetheless fired after insertion but before hydration occurs. If any of these tree events incur a read of the tree location where a proxy is pending hydration, then a new proxy will be created before hydration of the existing proxy occurs. When hydration of that proxy does occur soon after, it will throw an error because it is illegal to map two proxies to the same input node.

Fixing this requires careful consideration of the event sequencing. This PR implements the following solution. A new anchor event is introduced that fires immediately after content is inserted (and before any other events are fired). This event, childrenChanged is a mirror of the existing childrenChanging event; it happens just after the edit to the forest rather than just before. An event in the editable tree layer is built on top of this. However, it is problematic if anyone subscribes to such an event. It can introduce the same problem as exists now: if the event listener reads the tree in the wrong place during an insert, proxy hydration will fail for the same reason. So, the event registration is not exposed publicly alongside other events on TreeNode but is instead keyed by an internal (un-exported) symbol (onNextChange). It is further tailored to the scenario at hand; the event can only have one listener at a time and it automatically deregisters itself after firing the first time. This makes it suitable for proxy hydration and not much more, so as to discourage any more internal widespread use. Anytime that content is inserted into the tree by the proxy API, this event is subscribed to just before the insertion, and proxy hydration occurs when the event fires. Proxies are thus hydrated before any other events can examine to the tree state after the change.

There are other ways to solve this problem. For example, if inserted content could be identified with some token (e.g. a node key a.k.a. a node identifier), then the proxies could be keyed off of the token rather than the edit node, and be cached in a map even before insertion. Such a token does not currently exist but may in the future, at which time we can revisit this.

This PR also does some minor test and documentation cleanup.

[msfluid-bot]

⯅ @fluid-example/bundle-size-tests: +729 Bytes

Metric Name	Baseline Size	Compare Size	Size Diff
aqueduct.js	447.51 KB	447.51 KB	■ No change
connectionState.js	680 Bytes	680 Bytes	■ No change
containerRuntime.js	240.61 KB	240.61 KB	■ No change
loader.js	178.94 KB	178.94 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	274.97 KB	275.68 KB	⯅ +729 Bytes
Total Size	1.73 MB	1.74 MB	⯅ +729 Bytes

---

Baseline commit: 4505170

Generated by 🚫 dangerJS against 2212ee1

[ChumpChief]

@tyler-cai-microsoft from the description, sounds like this is the issue you repro'd?

[noencke]

@tyler-cai-microsoft from the description, sounds like this is the issue you repro'd?

The error thrown is is "Cannot associate an edit node with multiple targets".

[ChumpChief]

@tyler-cai-microsoft from the description, sounds like this is the issue you repro'd?

The error thrown is is "Cannot associate an edit node with multiple targets".

Tyler's repro is here if you'd like to try it against your change: https://github.com/microsoft/FluidFramework/pull/18232/files#diff-c97fa3e4c7401e9a6829502d0aad7df0c10e5289fd73315465254f0c429be6d0R12

[noencke]

@tyler-cai-microsoft from the description, sounds like this is the issue you repro'd?

The error thrown is is "Cannot associate an edit node with multiple targets".

Tyler's repro is here if you'd like to try it against your change: https://github.com/microsoft/FluidFramework/pull/18232/files#diff-c97fa3e4c7401e9a6829502d0aad7df0c10e5289fd73315465254f0c429be6d0R12

Thanks for the repro! Yea, that is the exact bug this PR is fixing. I confirmed that that test passes with the code in this PR.