mengw15 commented on code in PR #8581:
URL: https://github.com/apache/texera/pull/8581#discussion_r4057132871
##########
frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts:
##########
@@ -255,7 +263,17 @@ export class WorkflowEditorComponent implements OnInit,
AfterViewInit, OnDestroy
ngOnDestroy(): void {
this.paperResizeObserver?.disconnect();
- document.removeEventListener("keydown",
this._handleKeyboardAction.bind(this));
+ // The paper is bound to the joint graph, which is root-provided and
outlives this component,
+ // so an undisposed one goes on listening to that graph from a DOM node no
longer on the page.
+ // Harmless while every mount followed a page load; the switch between a
workflow's two views
+ // routes now, so a mount happens on every switch and the papers pile up.
Two live papers on
+ // one model both answer pointer events, and whichever answers decides
whether an operator can
+ // be dragged -- measured: an operator was undraggable after two
round-trips (issue #8582).
Review Comment:
This says more than the thread does. Your reply above retracts exactly this:
one operator measured by model id across six round-trips gave "one isolated
failure that did not recur", and the undraggable-operator symptom is "still
unconfirmed either way". The comment asserts both the mechanism (whichever
paper answers decides whether an operator can be dragged) and a measurement
(undraggable after two round-trips) as settled. The leak is real on its own and
does not need the symptom to justify disposing the paper; the thread will be
buried long before this line is, so it is worth saying only what held up — that
the papers pile up once remounting is the normal path, and that #8582 tracks
whether the dragging symptom follows from it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]