Fix removing the `forceReload` URL parameter
In change 347036 we have introduced a different way of removing URL
parameters that are just processed once and then ignored. Apparently
update the state in a subscription to the very same state is not a
good idea. It causes observables to be called in unpredictable order.
More specifically the state subscriber in `gr-router.ts` was called
without the `forceReload` parameter first and then with the parameter
afterwards.
Wrapping a `setTimeout()` around the `updateState()` call is not
beautiful, but seems appropriate and gets a nice long comment to
explain why it is there.
Release-Notes: skip
Google-Bug-Id: b/251396643
Change-Id: I3fa12ad93e1fbe4fdecd4f5d54ea65cf2543d9fc
diff --git a/polygerrit-ui/app/models/views/change.ts b/polygerrit-ui/app/models/views/change.ts
index 826ec66..a2d3506 100644
--- a/polygerrit-ui/app/models/views/change.ts
+++ b/polygerrit-ui/app/models/views/change.ts
@@ -155,11 +155,18 @@
super(undefined);
this.state$.subscribe(s => {
if (s?.usp || s?.forceReload || s?.openReplyDialog) {
- this.updateState({
- usp: undefined,
- forceReload: undefined,
- openReplyDialog: undefined,
- });
+ // We had been running into issues with directly calling
+ // `this.updateState()` without `setTimeout()`, because we want to
+ // first allow all subscribers to process the first state update before
+ // creating another one. For some unknown reason some subscribers even
+ // got the state updates out of order.
+ setTimeout(() => {
+ this.updateState({
+ usp: undefined,
+ forceReload: undefined,
+ openReplyDialog: undefined,
+ });
+ }, 0);
}
});
}
