Cancel all debouncers when the element is detached
The long living (60 seconds) debouncer from the error manager was most
likely causing major headaches by making the CI test suite extremely
flaky. Canceling on detach will hopefully make frontend developers much
happier again. :-)
All the other debouncers are not likely not cause major problems,
beacuse they are typically short lived, but it should be common best
practice to always cancel all debouncers on detach. So this change
fixes the entire code base.
There is a call to `flushDebouncers()` is the common-test-setup.ts,
which was thought to be a safeguard against the problem, but in fact
it is not. Added a commment explaining that.
Change-Id: I18f7a8e0304d3f75cf4317ea567b9f76b585398b
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
index ca3161f..e5396c3 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
@@ -168,6 +168,8 @@
};
}
+const DEBOUNCER_STORE = 'store';
+
@customElement('gr-reply-dialog')
export class GrReplyDialog extends KeyboardShortcutMixin(
GestureEventListeners(LegacyElementMixin(PolymerElement))
@@ -425,6 +427,11 @@
this.jsAPI.addElement(TargetElement.REPLY_DIALOG, this);
}
+ /** @override */
+ detached() {
+ this.cancelDebouncer(DEBOUNCER_STORE);
+ }
+
open(focusTarget?: FocusTarget) {
if (!this.change) throw new Error('missing required change property');
this.knownLatestState = LatestPatchState.CHECKING;
@@ -1332,7 +1339,7 @@
_draftChanged(newDraft: string, oldDraft?: string) {
this.debounce(
- 'store',
+ DEBOUNCER_STORE,
() => {
if (!newDraft.length && oldDraft) {
// If the draft has been modified to be empty, then erase the storage
