-
Notifications
You must be signed in to change notification settings - Fork 554
refactor(tree): Support pausing Tree events for table operations #25298
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 19 commits
d535c62
c36a3db
bca00a7
8967eba
d4a3e09
8d99ddd
55a69d9
53e0f0b
1292e55
c0f54ed
0dc1740
4a07c85
ac1cbbd
270401c
d694730
8d3526c
c29a0c8
9c83f13
bdfd345
8c93b42
1ead4e4
b5a61c4
724bb05
6329538
1d7116d
c4e10a4
cdf9853
2d874a4
3949ff8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -684,6 +684,24 @@ export class AnchorSet implements AnchorLocator { | |
} | ||
} | ||
|
||
// #region TODO: document all of this | ||
|
||
private eventsPaused: boolean = false; | ||
private bufferedEvents: BufferedEvent[] = []; | ||
|
||
public pauseEvents(): void { | ||
this.eventsPaused = true; | ||
this.bufferedEvents = []; | ||
} | ||
|
||
public resumeAndFlushEvents(): void { | ||
this.eventsPaused = false; | ||
flushBufferedEvents(this.bufferedEvents); | ||
this.bufferedEvents = []; | ||
} | ||
|
||
// #endregion | ||
|
||
/** | ||
* Provides a visitor that can be used to mutate this {@link AnchorSet}. | ||
* | ||
|
@@ -768,33 +786,10 @@ export class AnchorSet implements AnchorLocator { | |
} | ||
this.anchorSet.activeVisitor = undefined; | ||
|
||
// Aggregate changedFields by node. | ||
const eventsByNode: Map<PathNode, Set<FieldKey>> = new Map(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note for reviewers: extracted into a static function below so it can be reused. This functionality doesn't consume any object state, so no functional changes were required. |
||
for (const { node, event, changedField } of this.bufferedEvents) { | ||
if (event === "childrenChangedAfterBatch") { | ||
const keys = getOrCreate(eventsByNode, node, () => new Set()); | ||
keys.add( | ||
changedField ?? | ||
fail(0xb57 /* childrenChangedAfterBatch events should have a changedField */), | ||
); | ||
} | ||
} | ||
|
||
const alreadyEmitted = new Map<PathNode, (keyof AnchorEvents)[]>(); | ||
for (const { node, event } of this.bufferedEvents) { | ||
const emittedEvents = getOrAddEmptyToMap(alreadyEmitted, node); | ||
if (emittedEvents.includes(event)) { | ||
continue; | ||
} | ||
emittedEvents.push(event); | ||
if (event === "childrenChangedAfterBatch") { | ||
const changedFields = | ||
eventsByNode.get(node) ?? | ||
fail(0xaeb /* childrenChangedAfterBatch events should have changedFields */); | ||
node.events.emit(event, { changedFields }); | ||
} else { | ||
node.events.emit(event); | ||
} | ||
if (this.anchorSet.eventsPaused) { | ||
this.anchorSet.bufferedEvents.push(...this.bufferedEvents); | ||
} else { | ||
flushBufferedEvents(this.bufferedEvents); | ||
} | ||
}, | ||
notifyChildrenChanging(): void { | ||
|
@@ -945,6 +940,40 @@ export class AnchorSet implements AnchorLocator { | |
} | ||
} | ||
|
||
/** | ||
* Flushes and emits any buffered events. | ||
*/ | ||
function flushBufferedEvents(bufferedEvents: readonly BufferedEvent[]): void { | ||
// Aggregate changedFields by node. | ||
const eventsByNode: Map<PathNode, Set<FieldKey>> = new Map(); | ||
for (const { node, event, changedField } of bufferedEvents) { | ||
if (event === "childrenChangedAfterBatch") { | ||
const keys = getOrCreate(eventsByNode, node, () => new Set()); | ||
keys.add( | ||
changedField ?? | ||
fail(0xb57 /* childrenChangedAfterBatch events should have a changedField */), | ||
); | ||
} | ||
} | ||
|
||
const alreadyEmitted = new Map<PathNode, (keyof AnchorEvents)[]>(); | ||
for (const { node, event } of bufferedEvents) { | ||
const emittedEvents = getOrAddEmptyToMap(alreadyEmitted, node); | ||
if (emittedEvents.includes(event)) { | ||
continue; | ||
} | ||
emittedEvents.push(event); | ||
if (event === "childrenChangedAfterBatch") { | ||
const changedFields = | ||
eventsByNode.get(node) ?? | ||
fail(0xaeb /* childrenChangedAfterBatch events should have changedFields */); | ||
node.events.emit(event, { changedFields }); | ||
} else { | ||
node.events.emit(event); | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Indicates the status of a `NodePath`. | ||
*/ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -219,9 +219,7 @@ export class TreeNodeKernel { | |
for (const eventName of kernelEvents) { | ||
if (events.hasListeners(eventName)) { | ||
this.#hydrationState.offAnchorNode.add( | ||
// Argument is forwarded between matching events, so the type should be correct. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note for reviewers: this explicit typing as |
||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
anchorNode.events.on(eventName, (arg: any) => events.emit(eventName, arg)), | ||
anchorNode.events.on(eventName, (arg) => events.emit(eventName, arg)), | ||
); | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: revert this