diff --git a/.serena/memories/frontend/workspace-state-persistence-subtleties.md b/.serena/memories/frontend/workspace-state-persistence-subtleties.md index eee6de48b8..594df35e2d 100644 --- a/.serena/memories/frontend/workspace-state-persistence-subtleties.md +++ b/.serena/memories/frontend/workspace-state-persistence-subtleties.md @@ -20,6 +20,12 @@ - `commit-changes` refuses to create commits unless `:permissions :can-edit` is true. It captures file revn/vern, selected-before, features, tags, undo group, and translation flag into a `::commit` event. A refusal answers with an empty stream, never nil, so callers subscribe uniformly instead of branching on nil. - Applying a remote commit first rolls back pending local commits, applies the remote changes, then replays pending local redo changes. Index updates are emitted for undo, remote redo, and replayed redo paths. - Local commits are independently consumed by undo, persistence, WASM model updates, thumbnail/library watchers, and text position-data recalculation. +- `dch/changes-applied?` follows local application and carries the commit + with its prior file data. The component watcher syncs each transition + before the next user commit, keeping undo groups adjacent. Its barrier + remains open through the event turn. Foreign file cascades stay in common. +- Layout batches cross an `:asap` boundary before reflow so synchronous + component sync cannot emit into the buffer being closed and lose a task. - Persistence buffers local commits: status becomes pending after about 200ms, commits are flushed after about 3s or `::force-persist`, and buffered commits are merged per file before `:update-file`. - Persistence sends revn as the max of the commit revn and locally tracked latest revn; remote commits update that revn tracker. - Persistence is skipped in version preview/read-only mode or without edit permission. diff --git a/frontend/src/app/main/data/changes.cljs b/frontend/src/app/main/data/changes.cljs index ee8eaa33f3..5e9f35be91 100644 --- a/frontend/src/app/main/data/changes.cljs +++ b/frontend/src/app/main/data/changes.cljs @@ -35,6 +35,9 @@ (def commit? (ptk/type? ::commit)) +(def changes-applied? + (ptk/type? ::changes-applied)) + (defn- fix-page-id "For events that modifies the page, page-id does not comes as a property so we assign it from the `id` property." @@ -203,6 +206,9 @@ (get-pending-commits state))] (rx/concat (rx/of (apply-changes-localy commit pending)) + (rx/of (ptk/data-event ::changes-applied + {:commit commit + :old-data (get-in state [:files file-id :data])})) (if pending (rx/concat (->> (rx/from (reverse pending)) diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs index 6f7cde8c23..0b973ccaa3 100644 --- a/frontend/src/app/main/data/workspace/libraries.cljs +++ b/frontend/src/app/main/data/workspace/libraries.cljs @@ -57,7 +57,6 @@ [app.main.features :as features] [app.main.features.pointer-map :as fpmap] [app.main.fonts :as fonts] - [app.main.refs :as refs] [app.main.repo :as rp] [app.main.router :as rt] [app.main.store :as st] @@ -1399,31 +1398,19 @@ (rx/filter (fn [event-type] (or (= ::dwpg/finalize-page event-type) (= ::watch-component-changes event-type))))) - workspace-data-s - (->> (rx/from-atom refs/workspace-data {:emit-current-value? true}) - (rx/share)) - - ;; Pair every commit with the file data right before it - ;; (so deleted shapes still exist when we inspect the change). - workspace-buffer-s - (->> (rx/concat - (rx/take 1 workspace-data-s) - workspace-data-s) - (rx/buffer 2 1) - (rx/map first)) - - ;; Barriers open before async inspection and close after detection. + ;; Barriers cover detection and the derived work of each commit. ;; Every local commit opens one, so a wait covers the watcher's ;; decision for any commit: sync, only touch, or drop. pending-sync-barriers* (atom #{}) start-sync-barrier - (fn [{:keys [file-id] :as event}] - (let [task (when (uuid? file-id) - (wrf/start! :sync-file [file-id]))] + (fn [{:keys [commit old-data]}] + (let [file-id (:file-id commit) + task (when (uuid? file-id) + (wrf/start! :sync-file [file-id]))] (when task (swap! pending-sync-barriers* conj task)) - [event task])) + [commit task old-data])) finish-sync-barrier! (fn [task] @@ -1431,17 +1418,11 @@ (wrf/finish! task) (swap! pending-sync-barriers* disj task))) - ;; Each batch entry is [[event task] old-data]. - finish-batch-barriers! - (fn [batch] - (doseq [[[_ task] _] batch] - (finish-sync-barrier! task))) - local-commits-s (->> stream - (rx/filter dch/commit?) + (rx/filter dch/changes-applied?) (rx/map deref) - (rx/filter #(= :local (:source %))) + (rx/filter #(= :local (get-in % [:commit :source]))) ;; Keep waits pending while component changes are checked. (rx/map start-sync-barrier) (rx/share)) @@ -1451,10 +1432,10 @@ ;; corrective commits (font-load selrect fix, position-data regen) ;; are not user component edits. dropped-commit? - (fn [[{:keys [translation? skip-component-sync? changes]} _]] + (fn [[{:keys [translation? skip-component-sync? changes]} _ old-data]] (or skip-component-sync? (and translation? - (moves-whole-mains? @refs/workspace-data changes)))) + (moves-whole-mains? old-data changes)))) dropped-commits-s (->> local-commits-s @@ -1465,22 +1446,13 @@ commits-s (->> local-commits-s (rx/filter (complement dropped-commit?)) - ;; Paired as it arrives, so later commits in the same batch - ;; cannot hide the shapes this one moved or deleted. - (rx/with-latest-from workspace-buffer-s) (rx/share)) - ;; Buffers commits until a timer turn passes with no new ones. The - ;; debounce timer (not a microtask) queues behind any timers already - ;; due, so a burst of commits is inspected as one batch in one task. - commit-batches-s - (rx/buffer-until (rx/debounce 0 commits-s) commits-s) - get-component-changes (fn [old-data {:keys [file-id changes save-undo? undo-group]}] (let [changed-components (when (and old-data - (or (nil? file-id) (= file-id (:id old-data)))) + (= file-id (:current-file-id state) (:id old-data))) (into #{} (mapcat (partial ch/components-changed old-data)) changes))] @@ -1500,14 +1472,11 @@ ;; Undos only bump :modified-at. (map #(vector ::touch-component %) changed-components)))) - ;; One event per distinct component change in the batch, each - ;; commit checked against the file data right before it + ;; Sync each transition before a later user commit can replace its + ;; state or separate its sync from its undo group. get-component-events - (fn [batch] - (->> batch - (mapcat (fn [[[event _] old-data]] - (get-component-changes old-data event))) - (distinct) + (fn [event old-data] + (->> (get-component-changes old-data event) (map (fn [[type component-id file-id undo-group]] (if (= type ::component-changed) (component-changed component-id file-id undo-group) @@ -1515,11 +1484,17 @@ (rx/from))) component-events-s - (->> commit-batches-s - (rx/mapcat - (fn [batch] - (->> (get-component-events batch) - (rx/finalize #(finish-batch-barriers! batch))))) + (->> commits-s + ;; Barrier completion is async; it must not queue later + ;; commits behind the previous transition. + (rx/merge-map + (fn [[event task old-data]] + (->> (rx/concat + (get-component-events event old-data) + ;; Keep the completion barrier through the current + ;; event turn, including derived layout work. + (->> (rx/of nil) (rx/observe-on :async) (rx/ignore))) + (rx/finalize #(finish-sync-barrier! task))))) ;; Close barriers left behind when the page shuts down. (rx/finalize #(wrf/finish-tasks! @pending-sync-barriers*)) (rx/share)) diff --git a/frontend/src/app/main/data/workspace/shape_layout.cljs b/frontend/src/app/main/data/workspace/shape_layout.cljs index 16d9bc027b..0983e2c523 100644 --- a/frontend/src/app/main/data/workspace/shape_layout.cljs +++ b/frontend/src/app/main/data/workspace/shape_layout.cljs @@ -116,6 +116,9 @@ (rx/map #(assoc % ::reflow-task (wrf/start! :layout (:ids %)))) (rx/buffer-time 100) (rx/filter #(d/not-empty? %)) + ;; Open the next buffer before a reflow's synchronous component + ;; sync can request more layout work. + (rx/observe-on :asap) ;; The updates of a page reflow together, in the undo group of ;; the step that asked for them: the one an update names, as ;; those with none come from the same step. diff --git a/frontend/test/frontend_tests/composable_tests/golden_master.edn b/frontend/test/frontend_tests/composable_tests/golden_master.edn index c2cadbeddf..cae54ad46f 100644 --- a/frontend/test/frontend_tests/composable_tests/golden_master.edn +++ b/frontend/test/frontend_tests/composable_tests/golden_master.edn @@ -30,7 +30,7 @@ {:transcript "one-of -> \n {:target :copy-root, :value 500}", :states ["000943ecb6c266ee" "209fe67d76a27a9b"]} {:transcript "one-of -> \n one-of -> ", :states ["94318a628db305b6"]}] :case-aj-reflow-after-nested-sync-is-its-own-undo-step - [{:transcript " {:component \"main\", :id sync-main-component}\n {:component \"main\", :level 0, :outer sync-main-outercomp-0}\n {:component \"main\", :level 1, :outer sync-main-outercomp-1}\n {:component \"main\", :copy sync-main-copyhead-0}\n {:target #fn, :property :fills, :value \"#0000ff\"}\n {:times 1}", :states ["43ca8b2de80a51fc" "32b45a5ac7536b08" "7104bfaccd42cd3b"]}] + [{:transcript " {:component \"main\", :id sync-main-component}\n {:component \"main\", :level 0, :outer sync-main-outercomp-0}\n {:component \"main\", :level 1, :outer sync-main-outercomp-1}\n {:component \"main\", :copy sync-main-copyhead-0}\n {:target #fn, :property :fills, :value \"#0000ff\"}\n {:times 1}", :states ["43ca8b2de80a51fc" "ce4ef0e73faa0894" "ef4de3d8d773582c"]}] :case-ak-update-main-pushes-grid-cells [{:transcript " {:target :copy-child, :to :other, :keep-touched? false}\n {:board :copy-root, :child :copy-child, :row 1, :column 3}\n {:target :copy-root}", :states ["5a6af6c0b8f86ac3" "278e4ff6bf0df746" "322e03c88f4a9dce" "432cfef7e9b05164"]} {:transcript "one-of -> \n {:board :copy-root, :child :copy-child, :row 1, :column 3}\n {:target :copy-root}", :states ["9900a7f71dc0cb5f" "de66ab1c3d4cc157" "6b5ff9fd86e2aa87"]}] @@ -205,7 +205,7 @@ {:transcript " {:component \"main\", :id sync-main-component}\n {:component \"main\", :level 0, :outer sync-main-outercomp-0}\n {:component \"main\", :copy sync-main-copyhead-0}\n {:target #fn, :property :fills, :value \"#0000ff\"}\n {:target #fn, :property :fills, :value \"#00ff00\"}\n {:target #fn, :property :fills, :value \"#aaaaaa\"}\n {:times 3}", :states ["08a251a3907c6037" "3dff1bfab5a5b77d" "b5df4ace7209f759" "cd9d65b6e9e44dd4" "ddea124a9ab145f1"]} {:transcript " {:component \"main\", :id sync-main-component}\n {:component \"main\", :level 1, :outer sync-main-outercomp-1}\n {:component \"main\", :level 1, :outer sync-main-outercomp-1}\n {:component \"main\", :copy sync-main-copyhead-0}\n {:target #fn, :property :fills, :value \"#0000ff\"}\n {:target #fn, :property :fills, :value \"#00ff00\"}\n {:target #fn, :property :fills, :value \"#aaaaaa\"}\n {:times 3}", :states ["4220f3cd46b6b642" "180cc6a41d003bb5" "9fa58862bf863db6" "3f2266fbef201c71" "32c65c945efec2cd"]}] :case-x-undo-before-sync-leaves-copies-unchanged - [{:transcript " {:target :main-instance, :property :fills, :value \"#ff0000\"}", :states ["734ec524d88eb6bb" "ff1fa863b88027e5"]}] + [{:transcript " {:target :main-instance, :property :fills, :value \"#ff0000\"}", :states ["c6423d10a4e5e278" "ac9279bfdfb5c7e5"]}] :case-y-text-override-merges-with-main-text-change [{:transcript "one-of -> \n {:target :copy-instance, :edit :text, :value \"copy text\"}\n {:target :main-instance, :edit :text, :value \"main text\"}", :states ["7ea87003598e5626" "f823f8147e941701" "3a6ecb19a9315cb8"]} {:transcript "one-of -> \n {:target :copy-instance, :edit :text, :value \"copy text\"}\n {:target :main-instance, :edit :font-size, :value \"24\"}", :states ["7ea87003598e5626" "bc06b54fa303bde9" "230d1c77384fc557"]} diff --git a/frontend/test/frontend_tests/data/workspace_sync_barrier_test.cljs b/frontend/test/frontend_tests/data/workspace_sync_barrier_test.cljs index 97b6a0001b..789d36da50 100644 --- a/frontend/test/frontend_tests/data/workspace_sync_barrier_test.cljs +++ b/frontend/test/frontend_tests/data/workspace_sync_barrier_test.cljs @@ -15,6 +15,7 @@ [app.config :as cf] [app.main.data.workspace.libraries :as dwl] [app.main.data.workspace.reflow :as wrf] + [app.main.data.workspace.undo :as dwu] [app.main.store :as st] [app.plugins.reflow :as pwrf] [beicon.v2.core :as rx] @@ -27,6 +28,22 @@ (def ^:private red "#ff0000") +(t/deftest ^:async consecutive-main-edits-undo-and-redo-with-their-copies + (ftm/install! (setup/simple-component-with-labeled-copy)) + (st/emit! (ftm/fill-commit :main-child red {})) + (st/emit! (ftm/fill-commit :main-child "#00ff00" {})) + (st/emit! (ftm/fill-commit :main-child "#0000ff" {})) + (t/is (= :settled (await (ftm/await-step [])))) + (doseq [[event color] [[dwu/undo "#00ff00"] + [dwu/undo red] + [dwu/undo setup/labeled-fill] + [dwu/redo red] + [dwu/redo "#00ff00"] + [dwu/redo "#0000ff"]]] + (t/is (= :settled (await (ftm/await-step [event])))) + (t/is (= color (ftm/fill-of :main-child))) + (t/is (= color (ftm/fill-of :copy-child))))) + (defn- sync-pending? [file-id] (contains? (get (wrf/pending) file-id) :sync-file))