From 90afb2a1eef9c02a5e624e03205ad4d9c80803d7 Mon Sep 17 00:00:00 2001
From: zadam <zadam.apps@gmail.com>
Date: Sun, 8 Sep 2019 09:40:29 +0200
Subject: [PATCH] improvements in widgets rendering - individual widget render
 is now async while composing them together to the sidebar is sync which
 solves some race problems

---
 src/public/javascripts/entities/note_short.js |  3 ++
 src/public/javascripts/services/sidebar.js    | 31 ++++++++++++-------
 .../javascripts/widgets/standard_widget.js    |  5 +--
 3 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/src/public/javascripts/entities/note_short.js b/src/public/javascripts/entities/note_short.js
index 4ecf5e0be..38411c508 100644
--- a/src/public/javascripts/entities/note_short.js
+++ b/src/public/javascripts/entities/note_short.js
@@ -7,6 +7,9 @@ const RELATION = 'relation';
 const RELATION_DEFINITION = 'relation-definition';
 
 /**
+ * FIXME: rethink how attributes are cached in Note entities since they are long lived inside the cache.
+ * Attribute cache should be limited to "transaction".
+ *
  * This note's representation is used in note tree and is kept in TreeCache.
  */
 class NoteShort {
diff --git a/src/public/javascripts/services/sidebar.js b/src/public/javascripts/services/sidebar.js
index 3985a2cb0..85be94fe7 100644
--- a/src/public/javascripts/services/sidebar.js
+++ b/src/public/javascripts/services/sidebar.js
@@ -52,14 +52,6 @@ class Sidebar {
             return;
         }
 
-        for (const widget of this.widgets) {
-            if (widget.cleanup) {
-                widget.cleanup();
-            }
-        }
-
-        this.widgets = [];
-
         const widgetClasses = (await Promise.all([
             import("../widgets/note_info.js"),
             import("../widgets/link_map.js"),
@@ -80,12 +72,14 @@ class Sidebar {
             widgetClasses.push(widgetClass);
         }
 
+        const widgets = [];
+
         for (const widgetClass of widgetClasses) {
             try {
                 const widget = new widgetClass(this.ctx, options, this.state);
 
                 if (await widget.isEnabled()) {
-                    this.widgets.push(widget);
+                    widgets.push(widget);
                 }
             }
             catch (e) {
@@ -93,16 +87,29 @@ class Sidebar {
             }
         }
 
+        this.renderWidgets(widgets);
+    }
+
+    // it's important that this method is sync so that the whole render-update is atomic
+    // otherwise we could get race issues (doubled widgets etc.)
+    renderWidgets(widgets) {
+        // cleanup old widgets
+        for (const widget of this.widgets) {
+            if (widget.cleanup) {
+                widget.cleanup();
+            }
+        }
+
+        this.widgets = widgets;
         this.widgets.sort((a, b) => a.getPosition() < b.getPosition() ? -1 : 1);
 
         const widgetsToAppend = [];
 
         for (const widget of this.widgets) {
             try {
-                const $el = await widget.render();
+                const $el = widget.render();
                 widgetsToAppend.push($el);
-            }
-            catch (e) {
+            } catch (e) {
                 ws.logError(`Error while rendering widget ${widget.widgetName}: ${e.message}`);
             }
         }
diff --git a/src/public/javascripts/widgets/standard_widget.js b/src/public/javascripts/widgets/standard_widget.js
index 5c9c866e6..6393ed369 100644
--- a/src/public/javascripts/widgets/standard_widget.js
+++ b/src/public/javascripts/widgets/standard_widget.js
@@ -43,7 +43,7 @@ class StandardWidget {
 
     getPosition() { return this.widgetOptions.position; }
 
-    async render() {
+    render() {
         const widgetId = `tab-${this.ctx.tabId}-widget-${this.widgetName}`;
 
         this.$widget = $(WIDGET_TPL);
@@ -73,7 +73,8 @@ class StandardWidget {
         this.$headerActions = this.$widget.find('.widget-header-actions');
         this.$headerActions.append(...this.getHeaderActions());
 
-        await this.renderBody();
+        // actual rendering is async
+        this.renderBody();
 
         return this.$widget;
     }