mirror of
https://github.com/penpot/penpot.git
synced 2026-10-29 14:00:37 +00:00
🐛 Hold the sync wait for every local commit
The component watcher opened its :sync-file wait only for commits saved to the undo stack. Undo and redo commit without it, so a wait such as the plugin waitForLayoutUpdate returned before the watcher had handled them. Translation and derived commits were filtered out before the wait opened. Open the wait for every local commit of a file and close it once the watcher has decided what to do: sync, only touch the component, or drop the commit. Dropped commits close it right away. Add wrf/pending, a snapshot of the pending work, for tests and diagnostics.
This commit is contained in:
parent
02c51f435d
commit
d3b9d82af2
@ -1404,11 +1404,13 @@
|
||||
(rx/map first))
|
||||
|
||||
;; Barriers open before async inspection and close after detection.
|
||||
;; 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 save-undo?] :as event}]
|
||||
(let [task (when (and save-undo? (uuid? file-id))
|
||||
(fn [{:keys [file-id] :as event}]
|
||||
(let [task (when (uuid? file-id)
|
||||
(wrf/start! :sync-file [file-id]))]
|
||||
(when task
|
||||
(swap! pending-sync-barriers* conj task))
|
||||
@ -1420,20 +1422,33 @@
|
||||
(wrf/finish! task)
|
||||
(swap! pending-sync-barriers* disj task)))
|
||||
|
||||
commits-s
|
||||
local-commits-s
|
||||
(->> stream
|
||||
(rx/filter dch/commit?)
|
||||
(rx/map deref)
|
||||
(rx/filter #(= :local (:source %)))
|
||||
;; Translation commits never propagate component changes.
|
||||
(rx/filter (complement :translation?))
|
||||
;; Derived / corrective commits (font-load selrect fix,
|
||||
;; position-data regen) are not user component edits.
|
||||
(rx/filter (complement :skip-component-sync?))
|
||||
;; Keep waits pending while component changes are checked.
|
||||
(rx/map start-sync-barrier)
|
||||
(rx/share))
|
||||
|
||||
;; Translation commits never propagate component changes. Derived /
|
||||
;; corrective commits (font-load selrect fix, position-data regen)
|
||||
;; are not user component edits.
|
||||
dropped-commit?
|
||||
(fn [[{:keys [translation? skip-component-sync?]} _]]
|
||||
(or translation? skip-component-sync?))
|
||||
|
||||
dropped-commits-s
|
||||
(->> local-commits-s
|
||||
(rx/filter dropped-commit?)
|
||||
(rx/tap (comp finish-sync-barrier! second))
|
||||
(rx/ignore))
|
||||
|
||||
commits-s
|
||||
(->> local-commits-s
|
||||
(rx/filter (complement dropped-commit?))
|
||||
(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.
|
||||
@ -1495,6 +1510,7 @@
|
||||
(when (or (contains? cf/flags :component-thumbnails)
|
||||
(features/active-feature? state "render-wasm/v1"))
|
||||
(->> (rx/merge
|
||||
dropped-commits-s
|
||||
component-events-s
|
||||
|
||||
;; WASM only: render the thumbnail on every component
|
||||
|
||||
@ -229,6 +229,11 @@
|
||||
(finish-tasks! (map tasks-by-id @remaining))
|
||||
(reset! remaining #{})))))))))
|
||||
|
||||
(defn pending
|
||||
"The work pending right now, as `{id -> {kind -> #{task-id}}}`."
|
||||
[]
|
||||
(.getValue ^js pending-shapes))
|
||||
|
||||
(defn settled
|
||||
"Observable that emits once every id in `ids` has drained from the pending
|
||||
map, then completes. A nil `ids` waits for every pending id; an empty one has
|
||||
|
||||
@ -245,7 +245,7 @@
|
||||
(map (fn [[lib-id lib]] [lib-id (assoc lib :library-of (:id file))]))
|
||||
aux))))))
|
||||
|
||||
(defn- current-file
|
||||
(defn current-file
|
||||
"Read the live workspace file out of the global store."
|
||||
[]
|
||||
(let [st @st/state]
|
||||
@ -380,23 +380,25 @@
|
||||
(set! st/state (:state original-store))
|
||||
(set! st/stream (:stream original-store)))
|
||||
|
||||
(defn install!
|
||||
"Install `situation` in the global store and start the watchers the workspace
|
||||
starts, replacing the previous variant's. Restores the global store first (a
|
||||
preceding namespace may have swapped it). Returns the situation."
|
||||
[situation]
|
||||
(restore-global-store!)
|
||||
(st/emit! (install-situation-event situation)
|
||||
(dw/finalize-edit-watchers)
|
||||
(dw/initialize-edit-watchers)
|
||||
(dwl/watch-library-changes))
|
||||
situation)
|
||||
|
||||
(defn- run-variant
|
||||
"Set up one variant on the global store and run its ops. `setup` returns a
|
||||
situation (file + roles). Restores the global store (a preceding namespace may
|
||||
have swapped it), installs the file + starts the watcher, then folds the ops.
|
||||
Calls `k` with the final situation."
|
||||
situation (file + roles). Calls `k` with the final situation."
|
||||
[setup ops k]
|
||||
;; fresh label space per variant (mirrors the pure `tm/run-all`)
|
||||
(cthi/reset-idmap!)
|
||||
(restore-global-store!)
|
||||
(let [situation (setup)]
|
||||
(st/emit! (install-situation-event situation))
|
||||
;; The same watchers the workspace starts; re-emitting them replaces the
|
||||
;; previous variant's.
|
||||
(st/emit! (dw/finalize-edit-watchers)
|
||||
(dw/initialize-edit-watchers)
|
||||
(dwl/watch-library-changes))
|
||||
(run-ops situation ops k)))
|
||||
(run-ops (install! (setup)) ops k))
|
||||
|
||||
;; --------------------------------------------------------------------------
|
||||
;; (3) Test-facing check
|
||||
|
||||
@ -0,0 +1,101 @@
|
||||
;; This Source Code Form is subject to the terms of the Mozilla Public
|
||||
;; License, v. 2.0. If a copy of the MPL was not distributed with this
|
||||
;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
|
||||
;;
|
||||
;; Copyright (c) KALEIDOS SUBSIDIARY SL
|
||||
|
||||
(ns frontend-tests.data.workspace-sync-barrier-test
|
||||
"Tests the `:sync-file` wait the component watcher holds for each local
|
||||
commit until it has decided what to do with it: sync, only touch the
|
||||
component, or drop the commit. Plugin `waitForLayoutUpdate` and the
|
||||
composable interpreter rely on it."
|
||||
(:require
|
||||
[app.common.files.changes-builder :as pcb]
|
||||
[app.common.logic.shapes :as cls]
|
||||
[app.common.test-helpers.composable.comp.setups :as setup]
|
||||
[app.common.test-helpers.files :as thf]
|
||||
[app.common.test-helpers.ids-map :as thi]
|
||||
[app.common.test-helpers.shapes :as ths]
|
||||
[app.main.data.changes :as dch]
|
||||
[app.main.data.workspace.reflow :as wrf]
|
||||
[app.main.store :as st]
|
||||
[app.plugins.reflow :as pwrf]
|
||||
[cljs.test :as t :include-macros true]
|
||||
[frontend-tests.composable-tests.interpreter :as ftm]))
|
||||
|
||||
;; The watcher schedules thumbnail renders that reach `window`, absent
|
||||
;; headless (see `ftm/install-thumbnail-noop!`).
|
||||
(t/use-fixtures :each
|
||||
{:before (fn []
|
||||
(thi/reset-idmap!)
|
||||
(ftm/install-thumbnail-noop!))
|
||||
:after ftm/restore-thumbnail!})
|
||||
|
||||
(def ^:private red "#ff0000")
|
||||
|
||||
(defn- main-child-fill-commit
|
||||
"A commit that sets the main child's fill to red, with `opts` merged into the
|
||||
commit params."
|
||||
[opts]
|
||||
(let [file (ftm/current-file)
|
||||
page (thf/current-page file)
|
||||
changes (cls/generate-update-shapes
|
||||
(pcb/empty-changes nil (:id page))
|
||||
#{(thi/id :main-child)}
|
||||
#(assoc % :fills (ths/sample-fills-color :fill-color red))
|
||||
(:objects page)
|
||||
{})]
|
||||
(dch/commit-changes (merge {:redo-changes (:redo-changes changes)
|
||||
:undo-changes (:undo-changes changes)}
|
||||
opts))))
|
||||
|
||||
(defn- sync-pending?
|
||||
[file-id]
|
||||
(contains? (get (wrf/pending) file-id) :sync-file))
|
||||
|
||||
(defn- fill-of
|
||||
[label]
|
||||
(-> (ths/get-shape (ftm/current-file) label) :fills first :fill-color))
|
||||
|
||||
(defn- run-commit
|
||||
"Install a component with a copy and emit the main child fill commit with
|
||||
`commit-opts`. Once the wait settles, call `check` with whether the sync
|
||||
wait was open right after the commit. Fails the test when the wait does not
|
||||
settle."
|
||||
[done commit-opts check]
|
||||
(ftm/install! (setup/simple-component-with-labeled-copy))
|
||||
(let [file-id (:id (ftm/current-file))]
|
||||
(st/emit! (main-child-fill-commit commit-opts))
|
||||
(let [open? (sync-pending? file-id)]
|
||||
(-> (pwrf/wait-for-layout-update nil 2000)
|
||||
(.then #(check open?)
|
||||
#(t/is false "the sync wait did not settle"))
|
||||
(.catch #(t/is false (str "the check threw: " %)))
|
||||
(.finally done)))))
|
||||
|
||||
(t/deftest user-edit-holds-the-wait-until-copies-sync
|
||||
(t/async done
|
||||
(run-commit done {}
|
||||
(fn [open?]
|
||||
(t/is open?)
|
||||
(t/is (= red (fill-of :copy-child)))))))
|
||||
|
||||
(t/deftest undo-commit-holds-the-wait-until-the-watcher-handles-it
|
||||
;; Undo and redo commit with `save-undo? false`. The watcher only touches
|
||||
;; the component for them, after a timer turn; the wait must cover that.
|
||||
(t/async done
|
||||
(run-commit done {:save-undo? false}
|
||||
(fn [open?]
|
||||
(t/is open?)))))
|
||||
|
||||
(t/deftest translation-commit-leaves-nothing-pending
|
||||
(t/async done
|
||||
(run-commit done {:translation? true}
|
||||
(fn [open?]
|
||||
(t/is (not open?))))))
|
||||
|
||||
(t/deftest derived-commit-leaves-nothing-pending
|
||||
(t/async done
|
||||
(run-commit done {:skip-component-sync? true}
|
||||
(fn [open?]
|
||||
(t/is (not open?))))))
|
||||
@ -35,6 +35,7 @@
|
||||
[frontend-tests.data.workspace-path-edition-test]
|
||||
[frontend-tests.data.workspace-reflow-test]
|
||||
[frontend-tests.data.workspace-shortcuts-test]
|
||||
[frontend-tests.data.workspace-sync-barrier-test]
|
||||
[frontend-tests.data.workspace-texts-test]
|
||||
[frontend-tests.data.workspace-thumbnails-test]
|
||||
[frontend-tests.data.workspace-versions-test]
|
||||
@ -189,6 +190,7 @@
|
||||
'frontend-tests.data.workspace-pages-test
|
||||
'frontend-tests.data.workspace-path-edition-test
|
||||
'frontend-tests.data.workspace-reflow-test
|
||||
'frontend-tests.data.workspace-sync-barrier-test
|
||||
'frontend-tests.data.workspace-shortcuts-test
|
||||
'frontend-tests.data.workspace-texts-test
|
||||
'frontend-tests.data.workspace-thumbnails-test
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user