From ac5f276585bf4f4e5092f4b0c8e95e2c1391ffcb Mon Sep 17 00:00:00 2001 From: "alonso.torres" Date: Fri, 9 Oct 2026 11:51:10 +0200 Subject: [PATCH] :recycle: Share the workspace edit watchers with the test harness The composable test interpreter wired its own copy of the undo stack subscription and started only the component watcher, so it reacted to edits differently from the app, and a missing subscription failed silently. Gather the subscriptions that react to edits while a file is open (undo stack, layout updates, text reflow, WASM text measurement) in initialize-edit-watchers, stopped by finalize-edit-watchers, and the per-page component and token watchers in watch-library-changes. The workspace and the interpreter both start them. Starting the edit watchers again replaces the running ones. A file reload starts the workspace again without finalizing it, which left two undo stack watchers running, so an edit added an extra undo entry. --- .../frontend/composable-component-tests.md | 13 +- frontend/src/app/main/data/workspace.cljs | 255 ++++++++++-------- .../app/main/data/workspace/libraries.cljs | 11 + .../src/app/main/data/workspace/pages.cljs | 3 +- .../composable_tests/interpreter.cljs | 38 +-- 5 files changed, 168 insertions(+), 152 deletions(-) diff --git a/.serena/memories/frontend/composable-component-tests.md b/.serena/memories/frontend/composable-component-tests.md index fc6774246e..76db2a2cbf 100644 --- a/.serena/memories/frontend/composable-component-tests.md +++ b/.serena/memories/frontend/composable-component-tests.md @@ -122,7 +122,9 @@ cross-level propagation requires progressively NESTED levels (one variant + plai nestings do not propagate between each other. **Interpreter:** installs the situation's files into the global `st/state` (aux files tagged -`:library-of`), starts the real `watch-component-changes` (+ harness `watch-undo-stack`), maps +`:library-of`), starts the production watchers the app starts — `dw/initialize-edit-watchers` +(undo stack, layout, text reflow, WASM text measurement; stopped by `dw/finalize-edit-watchers`) +and `dwl/watch-library-changes` (component + token watchers) — then maps event-ops to REAL workspace events (`dwsh/update-shapes`, `dwl/component-swap`, `dwv/variants-switch`, `dwt/increase-rotation` — which runs the `check-delta` placement classification — `dwt/update-dimensions`, `dwu/undo`, `dwl/sync-file`, …) and runs sync-ops' @@ -137,10 +139,11 @@ Running (frontend): `cd frontend && pnpm run build:test`, then `node target/tests/test.js --focus frontend-tests.composable-tests.comp.sync-test` (var-level focus for one case). -**Fidelity warning:** the harness drives a MINIMALLY-ASSEMBLED app — only some -`initialize-workspace` subscriptions are wired. Risk = SILENT UNDER-WIRING (e.g. undo needs the -harness `watch-undo-stack`). When a case needs app behaviour beyond a raw edit, check for an -unwired subscription and verify by PROBING store state, not by trusting a green assertion. +**Fidelity warning:** the harness drives a MINIMALLY-ASSEMBLED app: no file loading, no +persistence, no viewport. Edit-reacting subscriptions come from the shared events above; a NEW +edit-reacting subscription belongs inside `initialize-edit-watchers` (or +`watch-library-changes`), never wired inline in `initialize-workspace`, or the harness silently +misses it. Headless there is no DOM text measurement, so `:text/reflow` work never drains. **Caveats:** inline `Test` exceptions are UNCAUGHT on the frontend (crash the runner — assert in the trailing asserter). `(optional (in-sequence …))` is not flattened for the interpreter — use diff --git a/frontend/src/app/main/data/workspace.cljs b/frontend/src/app/main/data/workspace.cljs index 4838e89049..9e703a4bab 100644 --- a/frontend/src/app/main/data/workspace.cljs +++ b/frontend/src/app/main/data/workspace.cljs @@ -399,123 +399,38 @@ :file-id (dm/str file-id)) (rx/of (fetch-bundle file-id features)))))) -(defn initialize-workspace - [team-id file-id] - (assert (uuid? team-id) "expected valud uuid for `team-id`") - (assert (uuid? file-id) "expected valud uuid for `file-id`") - - (ptk/reify ::initialize-workspace - ptk/UpdateEvent - (update [_ state] - (-> state - (assoc :recent-colors (:recent-colors storage/user)) - (assoc :recent-fonts (:recent-fonts storage/user)) - (assoc :current-file-id file-id) - (assoc :workspace-presence {}) - (update :workspace-global dissoc :default-font) - (update :comments-local dcmt/merge-persisted-filters))) +(defn finalize-edit-watchers + "Stops the watchers started by `initialize-edit-watchers`." + [] + (ptk/reify ::finalize-edit-watchers + ptk/WatchEvent + (watch [_ _ _] + (rx/of (dwsl/finalize-shape-layout) + (dwtxt/finalize-text-reflow))))) +(defn initialize-edit-watchers + "Starts the watchers that react to edits while a file is open: the undo + stack, layout updates, text reflow and the WASM text measurements. They run + until `finalize-edit-watchers`, or until this event comes again (a file + reload), which replaces them. Tests that drive edits without the whole + workspace start them too, so they see the same reactions as the app." + [] + (ptk/reify ::initialize-edit-watchers ptk/WatchEvent (watch [_ state stream] - (let [stoper-s (rx/filter (ptk/type? ::finalize-workspace) stream) - rparams (rt/get-params state) - features (features/get-enabled-features state team-id) + (let [stopper-s (rx/merge + (rx/filter (ptk/type? ::finalize-edit-watchers) stream) + (rx/filter (ptk/type? ::initialize-edit-watchers) stream)) render-wasm-enabled? (features/active-feature? state "render-wasm/v1") render-wasm-ready? #(and render-wasm-enabled? (wasm-state/ready?))] - - (log/debug :hint "initialize-workspace" - :team-id (dm/str team-id) - :file-id (dm/str file-id)) - - (rx/concat + (rx/merge + ;; Stop the layout and text watchers a previous start left running. + (rx/of (dwsl/finalize-shape-layout) + (dwtxt/finalize-text-reflow) + (dwsl/initialize-shape-layout) + (dwtxt/initialize-text-reflow)) (->> (rx/merge - (rx/concat - ;; Fetch all essential data that should be loaded before the file - (rx/merge - (if ^boolean render-wasm-enabled? - (->> (rx/from @wasm/module) - (rx/filter true?) - (rx/tap (fn [_] - (let [event (ug/event "penpot:wasm:loaded")] - (ug/dispatch! event)))) - (rx/ignore)) - (rx/empty)) - - (->> stream - (rx/filter (ptk/type? ::df/fonts-loaded)) - (rx/take 1) - (rx/ignore)) - - (rx/of (ntf/hide) - (dcmt/retrieve-comment-threads file-id) - (dcmt/fetch-profiles) - (df/fetch-fonts team-id))) - - ;; Once the essential data is fetched, lets proceed to - ;; fetch the file bundle - (rx/of (initialize-file team-id file-id))) - - (->> stream - (rx/filter (ptk/type? ::bundle-fetched)) - (rx/take 1) - (rx/map deref) - (rx/mapcat - (fn [{:keys [file]}] - (log/debug :hint "bundle fetched" - :team-id (dm/str team-id) - :file-id (dm/str file-id)) - - (rx/of (dpj/initialize-project (:project-id file)) - (dwn/initialize team-id file-id) - (dwsl/initialize-shape-layout) - (dwtxt/initialize-text-reflow) - (fetch-libraries file-id features) - (-> (workspace-initialized file-id) - (with-meta {:team-id team-id - :file-id file-id})))))) - - ;; Install dev perf observers once the workspace is ready - (when (contains? cf/flags :perf-logs) - (->> stream - (rx/filter (ptk/type? ::workspace-initialized)) - (rx/take 1) - (rx/tap (fn [_] (perf/setup))))) - - (->> stream - (rx/filter (ptk/type? ::dps/persistence-notification)) - (rx/take 1) - (rx/map dwc/set-workspace-visited)) - - ;; Emit audit event with file statistics once all libraries are resolved - (->> stream - (rx/filter (ptk/type? ::all-libraries-resolved)) - (rx/take 1) - (rx/map #(emit-workspace-file-stats file-id team-id))) - - (when-let [component-id (some-> rparams :component-id uuid/parse)] - (->> stream - (rx/filter (ptk/type? ::workspace-initialized)) - (rx/observe-on :async) - (rx/take 1) - (rx/map #(dwl/go-to-local-component :id component-id :update-layout? (:update-layout rparams))))) - - (when (:board-id rparams) - (->> stream - (rx/filter (ptk/type? ::dwv/initialize-viewport)) - (rx/take 1) - (rx/map zoom-to-frame))) - - (when-let [comment-id (some-> rparams :comment-id uuid/parse)] - (->> stream - (rx/filter (ptk/type? ::workspace-initialized)) - (rx/observe-on :async) - (rx/take 1) - (rx/map #(dwcm/navigate-to-comment-id comment-id)))) - - ;; Keep comment thread positions in sync on undo/redo - (rx/of (dwcm/watch-comment-thread-position-changes stoper-s)) - ;; Resize auto-grow text shapes whose selrect does not match ;; the WASM text layout once their fonts finish loading. (->> stream @@ -578,7 +493,7 @@ :save-undo? false :tags #{:position-data} :skip-component-sync? true}))))) - (rx/take-until stoper-s))) + (rx/take-until stopper-s))) (->> stream (rx/filter dch/commit?) @@ -593,6 +508,121 @@ :selected-before selected-before}] (rx/of (dwu/append-undo entry stack-undo?))) (rx/empty)))))) + (rx/take-until stopper-s))))))) + +(defn initialize-workspace + [team-id file-id] + (assert (uuid? team-id) "expected valud uuid for `team-id`") + (assert (uuid? file-id) "expected valud uuid for `file-id`") + + (ptk/reify ::initialize-workspace + ptk/UpdateEvent + (update [_ state] + (-> state + (assoc :recent-colors (:recent-colors storage/user)) + (assoc :recent-fonts (:recent-fonts storage/user)) + (assoc :current-file-id file-id) + (assoc :workspace-presence {}) + (update :workspace-global dissoc :default-font) + (update :comments-local dcmt/merge-persisted-filters))) + + ptk/WatchEvent + (watch [_ state stream] + (let [stoper-s (rx/filter (ptk/type? ::finalize-workspace) stream) + rparams (rt/get-params state) + features (features/get-enabled-features state team-id) + render-wasm-enabled? (features/active-feature? state "render-wasm/v1")] + + (log/debug :hint "initialize-workspace" + :team-id (dm/str team-id) + :file-id (dm/str file-id)) + + (rx/concat + (->> (rx/merge + (rx/concat + ;; Fetch all essential data that should be loaded before the file + (rx/merge + (if ^boolean render-wasm-enabled? + (->> (rx/from @wasm/module) + (rx/filter true?) + (rx/tap (fn [_] + (let [event (ug/event "penpot:wasm:loaded")] + (ug/dispatch! event)))) + (rx/ignore)) + (rx/empty)) + + (->> stream + (rx/filter (ptk/type? ::df/fonts-loaded)) + (rx/take 1) + (rx/ignore)) + + (rx/of (ntf/hide) + (dcmt/retrieve-comment-threads file-id) + (dcmt/fetch-profiles) + (df/fetch-fonts team-id))) + + ;; Once the essential data is fetched, lets proceed to + ;; fetch the file bundle + (rx/of (initialize-file team-id file-id))) + + (->> stream + (rx/filter (ptk/type? ::bundle-fetched)) + (rx/take 1) + (rx/map deref) + (rx/mapcat + (fn [{:keys [file]}] + (log/debug :hint "bundle fetched" + :team-id (dm/str team-id) + :file-id (dm/str file-id)) + + (rx/of (dpj/initialize-project (:project-id file)) + (dwn/initialize team-id file-id) + (initialize-edit-watchers) + (fetch-libraries file-id features) + (-> (workspace-initialized file-id) + (with-meta {:team-id team-id + :file-id file-id})))))) + + ;; Install dev perf observers once the workspace is ready + (when (contains? cf/flags :perf-logs) + (->> stream + (rx/filter (ptk/type? ::workspace-initialized)) + (rx/take 1) + (rx/tap (fn [_] (perf/setup))))) + + (->> stream + (rx/filter (ptk/type? ::dps/persistence-notification)) + (rx/take 1) + (rx/map dwc/set-workspace-visited)) + + ;; Emit audit event with file statistics once all libraries are resolved + (->> stream + (rx/filter (ptk/type? ::all-libraries-resolved)) + (rx/take 1) + (rx/map #(emit-workspace-file-stats file-id team-id))) + + (when-let [component-id (some-> rparams :component-id uuid/parse)] + (->> stream + (rx/filter (ptk/type? ::workspace-initialized)) + (rx/observe-on :async) + (rx/take 1) + (rx/map #(dwl/go-to-local-component :id component-id :update-layout? (:update-layout rparams))))) + + (when (:board-id rparams) + (->> stream + (rx/filter (ptk/type? ::dwv/initialize-viewport)) + (rx/take 1) + (rx/map zoom-to-frame))) + + (when-let [comment-id (some-> rparams :comment-id uuid/parse)] + (->> stream + (rx/filter (ptk/type? ::workspace-initialized)) + (rx/observe-on :async) + (rx/take 1) + (rx/map #(dwcm/navigate-to-comment-id comment-id)))) + + ;; Keep comment thread positions in sync on undo/redo + (rx/of (dwcm/watch-comment-thread-position-changes stoper-s))) (rx/take-until stoper-s))))) @@ -626,8 +656,7 @@ (let [project-id (:current-project-id state)] (rx/of (dwn/finalize file-id) (dpj/finalize-project project-id) - (dwsl/finalize-shape-layout) - (dwtxt/finalize-text-reflow) + (finalize-edit-watchers) (dwcl/stop-picker) (dwc/set-workspace-visited) (modal/hide) diff --git a/frontend/src/app/main/data/workspace/libraries.cljs b/frontend/src/app/main/data/workspace/libraries.cljs index 6205eb3098..b9554c4aa4 100644 --- a/frontend/src/app/main/data/workspace/libraries.cljs +++ b/frontend/src/app/main/data/workspace/libraries.cljs @@ -1574,6 +1574,17 @@ (rx/map (fn [_] (sync-tokens-status-with-lib))) (rx/take-until stopper-s)))))) +(defn watch-library-changes + "Starts the watchers that propagate edits of the file's library while a page + is open: component changes to the copies, token changes to the tokens + status. They stop when the page is finalized." + [] + (ptk/reify ::watch-library-changes + ptk/WatchEvent + (watch [_ _ _] + (rx/of (watch-component-changes) + (watch-token-changes))))) + ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;; Backend interactions ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/frontend/src/app/main/data/workspace/pages.cljs b/frontend/src/app/main/data/workspace/pages.cljs index c8b8180c41..b4bb7ecc2a 100644 --- a/frontend/src/app/main/data/workspace/pages.cljs +++ b/frontend/src/app/main/data/workspace/pages.cljs @@ -113,8 +113,7 @@ (rx/empty) (rx/of (dwth/watch-state-changes file-id page-id))) - (rx/of (dwl/watch-component-changes)) - (rx/of (dwl/watch-token-changes)) + (rx/of (dwl/watch-library-changes)) (let [profile (:profile state) props (get profile :props)] diff --git a/frontend/test/frontend_tests/composable_tests/interpreter.cljs b/frontend/test/frontend_tests/composable_tests/interpreter.cljs index d7dcb238f0..4c1ea73af8 100644 --- a/frontend/test/frontend_tests/composable_tests/interpreter.cljs +++ b/frontend/test/frontend_tests/composable_tests/interpreter.cljs @@ -43,6 +43,7 @@ [app.common.test-helpers.ids-map :as cthi] [app.common.test-helpers.shapes :as cths] [app.main.data.changes :as dch] + [app.main.data.workspace :as dw] [app.main.data.workspace.libraries :as dwl] [app.main.data.workspace.shapes :as dwsh] [app.main.data.workspace.thumbnails :as dwth] @@ -361,36 +362,6 @@ (js/setTimeout continue grace) (continue))))))))) -(defn- watch-undo-stack - "Maintain the workspace UNDO STACK in the (global) store, mirroring the - production subscription in `app.main.data.workspace/initialize-workspace`: - for every local commit with `save-undo?` and non-empty `:undo-changes`, dispatch - `dwu/append-undo`. We start this in the harness because the minimal store setup - does not run the full `initialize-workspace` (which is where the real app wires - it). Re-emitting this event stops any previous instance (so re-install per - variant does not stack watchers)." - [] - (ptk/reify ::watch-undo-stack - ptk/WatchEvent - (watch [_ _ stream] - (let [stopper-s (->> stream (rx/filter (ptk/type? ::watch-undo-stack)))] - (->> stream - (rx/filter dch/commit?) - (rx/map deref) - (rx/filter #(= :local (:source %))) - (rx/mapcat - (fn [{:keys [save-undo? undo-changes redo-changes undo-group tags stack-undo? selected-before]}] - (if (and save-undo? (seq undo-changes)) - (rx/of (dwu/append-undo - {:undo-changes undo-changes - :redo-changes redo-changes - :undo-group undo-group - :tags tags - :selected-before selected-before} - stack-undo?)) - (rx/empty)))) - (rx/take-until stopper-s)))))) - (defonce ^:private original-store ;; Captured at namespace-LOAD time — i.e. before any test in the run executes. ;; Several test namespaces (the plugins suite) `set!` st/state / st/stream to @@ -420,8 +391,11 @@ (restore-global-store!) (let [situation (setup)] (st/emit! (install-situation-event situation)) - (st/emit! (dwl/watch-component-changes)) - (st/emit! (watch-undo-stack)) + ;; 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))) ;; --------------------------------------------------------------------------