enhance(perf): insert and delete blocks (#9142)

* enhance(perf): improve performance for both insert and delete

* fix: remember cursor pos before executing the body in a transaction

Otherwise, the edit-block and position could be changed

* fix: disable delete-concat when there's no child or right sibling

---------

Co-authored-by: Gabriel Horner <[email protected]>
Co-authored-by: Gabriel Horner <[email protected]>

.carve/ignore

@@ -82,3 +82,5 @@ logseq.graph-parser.nbb-test-runner/run-tests
 ;; For debugging
 frontend.fs.sync/debug-print-sync-events-loop
 frontend.fs.sync/stop-debug-print-sync-events-loop
+;; Used in macro
+frontend.state/get-current-edit-block-and-position

e2e-tests/editor.spec.ts

@@ -260,7 +260,7 @@ test('undo and redo after starting an action should not destroy text #6267', asy
 
   // And it should keep what was undone as a redo action
   await page.keyboard.press(modKey + '+Shift+z')
-  await expect(page.locator('text="text2"')).toHaveCount(1)
+  await expect(page.locator('text="text1 text2 [[]]"')).toHaveCount(1)
 })
 
 test('undo after starting an action should close the action menu #6269', async ({ page, block }) => {

src/main/frontend/components/block.cljs

@@ -927,8 +927,7 @@
         [:span.warning.mr-1 {:title "Block ref invalid"}
          (block-ref/->block-ref id)]))
   [:span.warning.mr-1 {:title "Block ref invalid"}
-    (block-ref/->block-ref id)]
-))
+    (block-ref/->block-ref id)]))
 
 (defn inline-text
   ([format v]

src/main/frontend/db/model.cljs

@@ -957,15 +957,8 @@ independent of format as format specific heading characters are stripped"
   "Doesn't include nested children."
   [repo block-uuid]
   (when-let [db (conn/get-db repo)]
-    (-> (d/q
-         '[:find [(pull ?b [*]) ...]
-           :in $ ?parent-id
-           :where
-           [?parent :block/uuid ?parent-id]
-           [?b :block/parent ?parent]]
-         db
-         block-uuid)
-        (sort-by-left (db-utils/entity [:block/uuid block-uuid])))))
+    (when-let [parent (db-utils/entity repo [:block/uuid block-uuid])]
+      (sort-by-left (:block/_parent parent) parent))))
 
 (defn get-block-children
   "Including nested children."

src/main/frontend/db/react.cljs

@@ -16,7 +16,7 @@
 ;;; keywords specs for reactive query, used by `react/q` calls
 ;; ::block
 ;; pull-block react-query
-(s/def ::block (s/tuple #(= ::block %) uuid?))
+(s/def ::block (s/tuple #(= ::block %) int?))
 ;; ::page-blocks
 ;; get page-blocks react-query
 (s/def ::page-blocks (s/tuple #(= ::page-blocks %) int?))

src/main/frontend/handler/block.cljs

@@ -6,7 +6,6 @@
    [frontend.db :as db]
    [frontend.db.model :as db-model]
    [frontend.db.react :as react]
-   [frontend.db.utils :as db-utils]
    [frontend.mobile.haptics :as haptics]
    [frontend.modules.outliner.core :as outliner-core]
    [frontend.modules.outliner.transaction :as outliner-tx]
@@ -70,14 +69,9 @@
                                    (util/distinct-by :db/id))))))
 
 (defn indentable?
-  [{:block/keys [parent] :as block}]
+  [{:block/keys [parent left]}]
   (when parent
-    (let [parent-block (db-utils/pull (:db/id parent))
-          first-child (first
-                       (db-model/get-block-immediate-children
-                        (state/get-current-repo)
-                        (:block/uuid parent-block)))]
-      (not= (:db/id block) (:db/id first-child)))))
+    (not= parent left)))
 
 (defn outdentable?
   [{:block/keys [level] :as _block}]

src/main/frontend/handler/editor.cljs

@@ -753,30 +753,28 @@
        (outliner-core/delete-blocks! [block] {:children? children?})))))
 
 (defn- move-to-prev-block
-  ([repo sibling-block format id value]
-   (move-to-prev-block repo sibling-block format id value true))
-  ([repo sibling-block format id value edit?]
-   (when (and repo sibling-block)
-     (when-let [sibling-block-id (dom/attr sibling-block "blockid")]
-       (when-let [block (db/pull repo '[*] [:block/uuid (uuid sibling-block-id)])]
-         (let [original-content (util/trim-safe (:block/content block))
-               value' (-> (property/remove-built-in-properties format original-content)
-                          (drawer/remove-logbook))
-               new-value (str value' value)
-               tail-len (count value)
-               pos (max
-                    (if original-content
-                      (gobj/get (utf8/encode original-content) "length")
-                      0)
-                    0)]
-           (when edit?
-             (edit-block! block pos id
-                          {:custom-content new-value
-                           :tail-len tail-len
-                           :move-cursor? false}))
-           {:prev-block block
-            :new-content new-value
-            :pos pos}))))))
+  [repo sibling-block format id value move?]
+  (when (and repo sibling-block)
+    (when-let [sibling-block-id (dom/attr sibling-block "blockid")]
+      (when-let [block (db/pull repo '[*] [:block/uuid (uuid sibling-block-id)])]
+        (let [original-content (util/trim-safe (:block/content block))
+              value' (-> (property/remove-built-in-properties format original-content)
+                         (drawer/remove-logbook))
+              new-value (str value' value)
+              tail-len (count value)
+              pos (max
+                   (if original-content
+                     (gobj/get (utf8/encode original-content) "length")
+                     0)
+                   0)
+              f (fn [] (edit-block! block pos id
+                                    {:custom-content new-value
+                                     :tail-len tail-len
+                                     :move-cursor? false}))]
+          (when move? (f))
+          {:prev-block block
+           :new-content new-value
+           :move-fn f})))))
 
 (declare save-block!)
 
@@ -802,7 +800,7 @@
                (when block-parent-id
                  (let [block-parent (gdom/getElement block-parent-id)
                        sibling-block (util/get-prev-block-non-collapsed-non-embed block-parent)
-                       {:keys [prev-block new-content]} (move-to-prev-block repo sibling-block format id value)
+                       {:keys [prev-block new-content move-fn]} (move-to-prev-block repo sibling-block format id value false)
                        concat-prev-block? (boolean (and prev-block new-content))
                        transact-opts (cond->
                                        {:outliner-op :delete-block}
@@ -812,7 +810,8 @@
                    (outliner-tx/transact! transact-opts
                      (when concat-prev-block?
                        (save-block! repo prev-block new-content))
-                     (delete-block-aux! block delete-children?))))))))))
+                     (delete-block-aux! block delete-children?))
+                   (move-fn)))))))))
    (state/set-editor-op! nil)))
 
 (defn delete-blocks!
@@ -829,7 +828,8 @@
         (move-to-prev-block repo sibling-block
                             (:block/format block)
                             (dom/attr sibling-block "id")
-                            "")))))
+                            ""
+                            true)))))
 
 (defn- set-block-property-aux!
   [block-or-id key value]
@@ -1962,7 +1962,7 @@
                   keep-uuid?]
            :or {exclude-properties []}}]
   (let [editing-block (when-let [editing-block (state/get-edit-block)]
-                        (some-> (db/pull (:db/id editing-block))
+                        (some-> (db/pull [:block/uuid (:block/uuid editing-block)])
                                 (assoc :block/content (state/get-edit-content))))
         has-unsaved-edits (and editing-block
                                (not= (:block/content (db/pull (:db/id editing-block)))
@@ -2425,8 +2425,9 @@
             :else
             (profile
              "Insert block"
-             (do (save-current-block!)
-                 (insert-new-block! state)))))))))
+             (outliner-tx/transact! {:outliner-op :insert-blocks}
+               (save-current-block!)
+               (insert-new-block! state)))))))))
 
 (defn- inside-of-single-block
   "When we are in a single block wrapper, we should always insert a new line instead of new block"
@@ -2580,15 +2581,18 @@
         ^js input (state/get-input)
         current-pos (cursor/pos input)
         value (gobj/get input "value")
-        right (outliner-core/get-right-node (outliner-core/block current-block))
+        right (outliner-core/get-right-sibling (:db/id current-block))
         current-block-has-children? (db/has-children? (:block/uuid current-block))
         collapsed? (util/collapsed? current-block)
         first-child (:data (tree/-get-down (outliner-core/block current-block)))
         next-block (if (or collapsed? (not current-block-has-children?))
-                     (:data right)
+                     (when right (db/pull (:db/id right)))
                      first-child)]
     (cond
-      (and collapsed? right (db/has-children? (tree/-get-id right)))
+      (nil? next-block)
+      nil
+
+      (and collapsed? right (db/has-children? (:block/uuid right)))
       nil
 
       (and (not collapsed?) first-child (db/has-children? (:block/uuid first-child)))

src/main/frontend/handler/events.cljs

@@ -608,10 +608,9 @@
               (plugin/open-waiting-updates-modal!))
             (plugin-handler/set-auto-checking! false))))))
 
-(defmethod handle :plugin/hook-db-tx [[_ {:keys [blocks tx-data tx-meta] :as payload}]]
+(defmethod handle :plugin/hook-db-tx [[_ {:keys [blocks tx-data] :as payload}]]
   (when-let [payload (and (seq blocks)
-                          (merge payload {:tx-data (map #(into [] %) tx-data)
-                                          :tx-meta (dissoc tx-meta :editor-cursor)}))]
+                          (merge payload {:tx-data (map #(into [] %) tx-data)}))]
     (plugin-handler/hook-plugin-db :changed payload)
     (plugin-handler/hook-plugin-block-changes payload)))
 

src/main/frontend/modules/editor/undo_redo.cljs

@@ -44,7 +44,7 @@
     [txs]
     (filterv (fn [[_ a & y]]
                (= :block/content a))
-      txs))
+             txs))
 
   (defn get-content-from-stack
     "For test."
@@ -60,22 +60,21 @@
     (when-let [stack @undo-stack]
       (when (seq stack)
         (let [removed-e (peek stack)
-              popped-stack (pop stack)
-              prev-e (peek popped-stack)]
+              popped-stack (pop stack)]
           (reset! undo-stack popped-stack)
-          [removed-e prev-e])))))
+          removed-e)))))
 
 (defn push-redo
   [txs]
   (let [redo-stack (get-redo-stack)]
-   (swap! redo-stack conj txs)))
+    (swap! redo-stack conj txs)))
 
 (defn pop-redo
   []
   (let [redo-stack (get-redo-stack)]
-   (when-let [removed-e (peek @redo-stack)]
-     (swap! redo-stack pop)
-     removed-e)))
+    (when-let [removed-e (peek @redo-stack)]
+      (swap! redo-stack pop)
+      removed-e)))
 
 (defn page-pop-redo
   [page-id]
@@ -119,7 +118,7 @@
                        (and redo? (not add?)) :db/retract
                        (and (not redo?) (not add?)) :db/add)]
               [op id attr value tx]))
-      txs)))
+          txs)))
 
 ;;;; Invokes
 
@@ -128,7 +127,7 @@
   (let [conn (conn/get-db false)]
     (d/transact! conn txs tx-meta)))
 
-(defn page-pop-undo
+(defn- page-pop-undo
   [page-id]
   (let [undo-stack (get-undo-stack)]
     (when-let [stack @undo-stack]
@@ -144,7 +143,7 @@
                   others (vec (concat before after))]
               (reset! undo-stack others)
               (prn "[debug] undo remove: " (nth stack idx'))
-              [(nth stack idx') others])))))))
+              (nth stack idx'))))))))
 
 (defn- smart-pop-undo
   []
@@ -154,56 +153,77 @@
       (pop-undo))
     (pop-undo)))
 
+(defn- set-editor-content!
+  "Prevent block auto-save during undo/redo."
+  []
+  (when-let [block (state/get-edit-block)]
+    (state/set-edit-content! (state/get-edit-input-id)
+                             (:block/content (db/entity (:db/id block))))))
+
+(defn- get-next-tx-editor-cursor
+  [tx-id]
+  (let [result (->> (sort (keys (:history/tx->editor-cursor @state/state)))
+                    (split-with #(not= % tx-id))
+                    second)]
+    (when (> (count result) 1)
+      (when-let [next-tx-id (nth result 1)]
+        (get-in @state/state [:history/tx->editor-cursor next-tx-id])))))
+
+(defn- get-previous-tx-id
+  [tx-id]
+  (let [result (->> (sort (keys (:history/tx->editor-cursor @state/state)))
+                    (split-with #(not= % tx-id))
+                    first)]
+    (when (>= (count result) 1)
+      (last result))))
+
+(defn- get-previous-tx-editor-cursor
+  [tx-id]
+  (when-let [prev-tx-id (get-previous-tx-id tx-id)]
+    (get-in @state/state [:history/tx->editor-cursor prev-tx-id])))
+
 (defn undo
   []
-  (let [[e prev-e] (smart-pop-undo)]
-    (when e
-      (let [{:keys [txs tx-meta]} e
-            new-txs (get-txs false txs)
-            undo-delete-concat-block? (and (= :delete-block (:outliner-op tx-meta))
-                                           (seq (:concat-data tx-meta)))
-            editor-cursor (cond
-                            undo-delete-concat-block?
-                            (let [data (:concat-data tx-meta)]
-                              (assoc (:editor-cursor e)
-                                     :last-edit-block {:block/uuid (:last-edit-block data)}
-                                     :pos (if (:end? data) :max 0)))
-
-                            ;; same block
-                            (= (get-in e [:editor-cursor :last-edit-block :block/uuid])
-                               (get-in prev-e [:editor-cursor :last-edit-block :block/uuid]))
-                            (:editor-cursor prev-e)
-
-                            :else
-                            (:editor-cursor e))]
-
-        (push-redo e)
-        (transact! new-txs (merge {:undo? true}
-                                  tx-meta
-                                  (select-keys e [:pagination-blocks-range])))
-
-        (when undo-delete-concat-block?
-          (when-let [block (state/get-edit-block)]
-            (state/set-edit-content! (state/get-edit-input-id)
-                                     (:block/content (db/entity (:db/id block))))))
-
-        (when (:whiteboard/transact? tx-meta)
-          (state/pub-event! [:whiteboard/undo e]))
-        (assoc e
-               :txs-op new-txs
-               :editor-cursor editor-cursor)))))
+  (when-let [e (smart-pop-undo)]
+    (let [{:keys [txs tx-meta tx-id]} e
+          new-txs (get-txs false txs)
+          current-editor-cursor (get-in @state/state [:history/tx->editor-cursor tx-id])
+          save-block? (= (:outliner-op tx-meta) :save-block)
+          prev-editor-cursor (get-previous-tx-editor-cursor tx-id)
+          editor-cursor (if (and save-block?
+                                 (= (:block/uuid (:last-edit-block prev-editor-cursor))
+                                    (:block/uuid (state/get-edit-block))))
+                          prev-editor-cursor
+                          current-editor-cursor)]
+      (push-redo e)
+      (transact! new-txs (merge {:undo? true}
+                                tx-meta
+                                (select-keys e [:pagination-blocks-range])))
+      (set-editor-content!)
+      (when (:whiteboard/transact? tx-meta)
+        (state/pub-event! [:whiteboard/undo e]))
+      (assoc e
+             :txs-op new-txs
+             :editor-cursor editor-cursor))))
 
 (defn redo
   []
-  (when-let [{:keys [txs tx-meta] :as e} (smart-pop-redo)]
-    (let [new-txs (get-txs true txs)]
+  (when-let [{:keys [txs tx-meta tx-id] :as e} (smart-pop-redo)]
+    (let [new-txs (get-txs true txs)
+          current-editor-cursor (get-in @state/state [:history/tx->editor-cursor tx-id])
+          editor-cursor (if (= (:outliner-op tx-meta) :save-block)
+                          current-editor-cursor
+                          (get-next-tx-editor-cursor tx-id))]
       (push-undo e)
       (transact! new-txs (merge {:redo? true}
                                 tx-meta
                                 (select-keys e [:pagination-blocks-range])))
+      (set-editor-content!)
       (when (:whiteboard/transact? tx-meta)
         (state/pub-event! [:whiteboard/redo e]))
-      (assoc e :txs-op new-txs))))
+      (assoc e
+             :txs-op new-txs
+             :editor-cursor editor-cursor))))
 
 (defn toggle-undo-redo-mode!
   []
@@ -231,14 +251,14 @@
                    #{:block/created-at :block/updated-at})))
     (reset-redo)
     (if (:replace? tx-meta)
-      (let [[removed-e _prev-e] (pop-undo)
+      (let [removed-e (pop-undo)
             entity (update removed-e :txs concat tx-data)]
         (push-undo entity))
       (let [updated-blocks (db-report/get-blocks tx-report)
-            entity {:blocks updated-blocks
+            entity {:tx-id (get-in tx-report [:tempids :db/current-tx])
+                    :blocks updated-blocks
                     :txs tx-data
                     :tx-meta tx-meta
-                    :editor-cursor (:editor-cursor tx-meta)
                     :pagination-blocks-range (get-in [:ui/pagination-blocks-range (get-in tx-report [:db-after :max-tx])] @state/state)
                     :app-state (select-keys @state/state
                                             [:route-match

src/main/frontend/modules/outliner/core.cljs

@@ -26,8 +26,14 @@
 
 (defn block
   [m]
-  (assert (map? m) (util/format "block data must be map, got: %s %s" (type m) m))
-  (->Block m))
+  (assert (or (map? m) (de/entity? m)) (util/format "block data must be map or entity, got: %s %s" (type m) m))
+  (if (de/entity? m)
+    (->Block {:db/id (:db/id m)
+              :block/uuid (:block/uuid m)
+              :block/page (:block/page m)
+              :block/left (:block/left m)
+              :block/parent (:block/parent m)})
+    (->Block m)))
 
 (defn get-data
   [block]
@@ -231,11 +237,6 @@
           children (db-model/get-block-immediate-children (state/get-current-repo) parent-id)]
       (map block children))))
 
-(defn get-right-node
-  [node]
-  {:pre [(tree/satisfied-inode? node)]}
-  (tree/-get-right node))
-
 (defn get-right-sibling
   [db-id]
   (when db-id
@@ -407,7 +408,7 @@
   (let [level-blocks (blocks-with-level blocks)]
     (filter (fn [b] (= 1 (:block/level b))) level-blocks)))
 
-(defn get-right-siblings
+(defn- get-right-siblings
   "Get `node`'s right siblings."
   [node]
   {:pre [(tree/satisfied-inode? node)]}
@@ -477,7 +478,7 @@
                         (:db/id target-block))
         get-new-id (fn [block lookup]
                      (cond
-                       (or (map? lookup) (vector? lookup))
+                       (or (map? lookup) (vector? lookup) (de/entity? lookup))
                        (when-let [uuid (if (and (vector? lookup) (= (first lookup) :block/uuid))
                                          (get uuids (last lookup))
                                          (get id->new-uuid (:db/id lookup)))]

src/main/frontend/modules/outliner/datascript.cljc

@@ -45,9 +45,14 @@
                                           v)))
                        x))))))
 
+#?(:cljs
+   (defn get-tx-id
+     [tx-report]
+     (get-in tx-report [:tempids :db/current-tx])))
+
 #?(:cljs
    (defn transact!
-     [txs opts]
+     [txs opts before-editor-cursor]
      (let [txs (remove-nil-from-transaction txs)
            txs (map (fn [m] (if (map? m)
                               (dissoc m
@@ -65,9 +70,9 @@
          (try
            (let [repo (get opts :repo (state/get-current-repo))
                  conn (conn/get-db repo false)
-                 editor-cursor (state/get-current-edit-block-and-position)
-                 meta (merge opts {:editor-cursor editor-cursor})
-                 rs (d/transact! conn txs (assoc meta :outliner/transact? true))]
+                 rs (d/transact! conn txs (assoc opts :outliner/transact? true))
+                 tx-id (get-tx-id rs)]
+             (swap! state/state assoc-in [:history/tx->editor-cursor tx-id] before-editor-cursor)
              (when true                 ; TODO: add debug flag
                (let [eids (distinct (mapv first (:tx-data rs)))
                      left&parent-list (->>

src/main/frontend/modules/outliner/transaction.cljc

@@ -27,7 +27,8 @@
   `(let [transact-data# frontend.modules.outliner.core/*transaction-data*
          opts# (if transact-data#
                  (assoc ~opts :nested-transaction? true)
-                 ~opts)]
+                 ~opts)
+         before-editor-cursor# (frontend.state/get-current-edit-block-and-position)]
      (if transact-data#
        (do ~@body)
        (binding [frontend.modules.outliner.core/*transaction-data* (transient [])]
@@ -40,7 +41,7 @@
                opts## (merge (dissoc opts# :additional-tx) tx-meta#)]
            (when (seq all-tx#) ;; If it's empty, do nothing
              (when-not (:nested-transaction? opts#) ; transact only for the whole transaction
-               (let [result# (frontend.modules.outliner.datascript/transact! all-tx# opts##)]
+               (let [result# (frontend.modules.outliner.datascript/transact! all-tx# opts## before-editor-cursor#)]
                  {:tx-report result#
                   :tx-data all-tx#
                   :tx-meta tx-meta#}))))))))

src/main/frontend/state.cljs

@@ -277,7 +277,9 @@
      :whiteboard/onboarding-tour?           (or (storage/get :whiteboard-onboarding-tour?) false)
      :whiteboard/last-persisted-at          {}
      :whiteboard/pending-tx-data            {}
-     :history/page-only-mode?               false})))
+     :history/page-only-mode?               false
+     ;; db tx-id -> editor cursor
+     :history/tx->editor-cursor             {}})))
 
 ;; Block ast state
 ;; ===============

src/main/logseq/api.cljs

@@ -688,11 +688,11 @@
                 block (if includeChildren
                         ;; nested children results
                         (first (outliner-tree/blocks->vec-tree
-                                 (db-model/get-block-and-children repo uuid) uuid))
+                                (db-model/get-block-and-children repo uuid) uuid))
                         ;; attached shallow children
                         (assoc block :block/children
-                                     (map #(list :uuid (get-in % [:data :block/uuid]))
-                                          (db/get-block-immediate-children repo uuid))))]
+                               (map #(list :uuid (:block/uuid %))
+                                 (db/get-block-immediate-children repo uuid))))]
             (bean/->js (sdk-utils/normalize-keyword-for-json block))))))))
 
 (def ^:export get_current_block
@@ -715,8 +715,9 @@
 (def ^:export get_next_sibling_block
   (fn [block-uuid]
     (when-let [block (db-model/query-block-by-uuid (sdk-utils/uuid-or-throw-error block-uuid))]
-      (when-let [right-siblings (outliner/get-right-siblings (outliner/->Block block))]
-        (bean/->js (sdk-utils/normalize-keyword-for-json (:data (first right-siblings))))))))
+      (when-let [right-sibling (outliner/get-right-sibling (:db/id block))]
+        (let [block (db/pull (:id right-sibling))]
+          (bean/->js (sdk-utils/normalize-keyword-for-json block)))))))
 
 (def ^:export set_block_collapsed
   (fn [block-uuid ^js opts]

src/test/frontend/db/model_test.cljs

@@ -163,6 +163,22 @@ foo:: bar"}])
                 (catch :default e
                   (ex-message e)))))))
 
+(deftest get-block-immediate-children
+  (load-test-files [{:file/path "pages/page1.md"
+                     :file/content "\n
+- parent
+  - child 1
+    - grandchild 1
+  - child 2
+    - grandchild 2
+  - child 3"}])
+  (let [parent (-> (d/q '[:find (pull ?b [*]) :where [?b :block/content "parent"]]
+                        (conn/get-db test-helper/test-db))
+                   ffirst)]
+    (is (= ["child 1" "child 2" "child 3"]
+           (map :block/content
+                (model/get-block-immediate-children test-helper/test-db (:block/uuid parent)))))))
+
 (deftest get-property-values
   (load-test-files [{:file/path "pages/Feature.md"
                      :file/content "type:: [[Class]]"}