From 5d56d28cb64deefb07aaa4516be5d0b692a8559d Mon Sep 17 00:00:00 2001 From: Andrey Antukh Date: Thu, 13 Feb 2025 17:04:34 +0100 Subject: [PATCH] :bug: Fix workspace hot reload race condtion (#5851) Mainly ensure that all required paramers for workspace file and page bootstrap are always available from parameters and not taken from context --- frontend/src/app/main/data/dashboard.cljs | 9 ++-- frontend/src/app/main/data/workspace.cljs | 33 +++++++-------- frontend/src/app/main/router.cljs | 5 +++ frontend/src/app/main/ui.cljs | 1 - frontend/src/app/main/ui/workspace.cljs | 41 +++++++------------ .../logic/copying_and_duplicating_test.cljs | 4 +- 6 files changed, 42 insertions(+), 51 deletions(-) diff --git a/frontend/src/app/main/data/dashboard.cljs b/frontend/src/app/main/data/dashboard.cljs index ddd2206b5..7883225ec 100644 --- a/frontend/src/app/main/data/dashboard.cljs +++ b/frontend/src/app/main/data/dashboard.cljs @@ -460,10 +460,11 @@ ptk/UpdateEvent (update [_ state] - (-> state - (assoc-in [:files id] file) - (assoc-in [:recent-files id] file) - (update-in [:projects project-id :count] inc))))) + (let [file (dissoc file :data)] + (-> state + (assoc-in [:files id] file) + (assoc-in [:recent-files id] file) + (update-in [:projects project-id :count] inc)))))) (defn create-file [{:keys [project-id name] :as params}] diff --git a/frontend/src/app/main/data/workspace.cljs b/frontend/src/app/main/data/workspace.cljs index ccffeef9a..b0ef81e1f 100644 --- a/frontend/src/app/main/data/workspace.cljs +++ b/frontend/src/app/main/data/workspace.cljs @@ -319,8 +319,6 @@ ptk/UpdateEvent (update [_ state] (-> state - (dissoc :files) - (dissoc :workspace-ready) (assoc :recent-colors (:recent-colors storage/user)) (assoc :recent-fonts (:recent-fonts storage/user)) (assoc :current-file-id file-id) @@ -395,11 +393,9 @@ (dissoc :current-file-id :workspace-editor-state - :files :workspace-media-objects :workspace-persistence :workspace-presence - :workspace-ready :workspace-undo) (update :workspace-global dissoc :read-only?) (assoc-in [:workspace-global :options-mode] :design))) @@ -427,18 +423,18 @@ (defmethod ptk/resolve ::reload-current-file [_ _] (reload-current-file)) (defn initialize-page - [page-id] - (assert (uuid? page-id) "expected valid uuid for `page-id`") + [file-id page-id] + (assert (uuid? file-id) "expected valid uuid for `file-id`") (ptk/reify ::initialize-page ptk/UpdateEvent (update [_ state] - (if-let [{:keys [id] :as page} (dsh/lookup-page state page-id)] + (if-let [page (dsh/lookup-page state file-id page-id)] ;; we maintain a cache of page state for user convenience with the exception of the ;; selection; when user abandon the current page, the selection is lost - (let [local (dm/get-in state [:workspace-cache id] default-workspace-local)] + (let [local (dm/get-in state [:workspace-cache [file-id page-id]] default-workspace-local)] (-> state - (assoc :current-page-id id) + (assoc :current-page-id page-id) (assoc :workspace-local (assoc local :selected (d/ordered-set))) (assoc :workspace-trimmed-page (dm/select-keys page [:id :name])) @@ -450,24 +446,25 @@ ptk/WatchEvent (watch [_ state _] - (if (dsh/lookup-page state page-id) - (let [file-id (:current-file-id state)] - (rx/of (preload-data-uris page-id) - (dwth/watch-state-changes file-id page-id) - (dwl/watch-component-changes))) - (rx/of (dcm/go-to-workspace)))))) + (if (dsh/lookup-page state file-id page-id) + (rx/of (preload-data-uris page-id) + (dwth/watch-state-changes file-id page-id) + (dwl/watch-component-changes)) + (rx/of (dcm/go-to-workspace :file-id file-id ::rt/replace true)))))) (defn finalize-page - [page-id] + [file-id page-id] + (assert (uuid? file-id) "expected valid uuid for `file-id`") (assert (uuid? page-id) "expected valid uuid for `page-id`") + (ptk/reify ::finalize-page ptk/UpdateEvent (update [_ state] (let [local (-> (:workspace-local state) (dissoc :edition :edit-path :selected)) - exit? (not= :workspace (dm/get-in state [:route :data :name])) + exit? (not= :workspace (rt/lookup-name state)) state (-> state - (update :workspace-cache assoc page-id local) + (update :workspace-cache assoc [file-id page-id] local) (dissoc :current-page-id :workspace-local :workspace-trimmed-page diff --git a/frontend/src/app/main/router.cljs b/frontend/src/app/main/router.cljs index 58e421983..e4c3b16a1 100644 --- a/frontend/src/app/main/router.cljs +++ b/frontend/src/app/main/router.cljs @@ -120,6 +120,11 @@ ([id params & {:as options}] (navigate id params options))) +(defn lookup-name + [state] + (dm/get-in state [:route :data :name])) + +;; FIXME: rename to lookup-params (defn get-params [state] (dm/get-in state [:route :params :query])) diff --git a/frontend/src/app/main/ui.cljs b/frontend/src/app/main/ui.cljs index bf6b1ebaa..71eca09e2 100644 --- a/frontend/src/app/main/ui.cljs +++ b/frontend/src/app/main/ui.cljs @@ -127,7 +127,6 @@ {::mf/props :obj ::mf/private true} [{:keys [team-id children]}] - (mf/with-effect [team-id] (st/emit! (dtm/initialize-team team-id)) (fn [] diff --git a/frontend/src/app/main/ui/workspace.cljs b/frontend/src/app/main/ui/workspace.cljs index a169ae616..24bf92e2e 100644 --- a/frontend/src/app/main/ui/workspace.cljs +++ b/frontend/src/app/main/ui/workspace.cljs @@ -9,7 +9,6 @@ (:require [app.common.data.macros :as dm] [app.main.data.common :as dcm] - [app.main.data.helpers :as dsh] [app.main.data.persistence :as dps] [app.main.data.plugins :as dpl] [app.main.data.workspace :as dw] @@ -43,13 +42,6 @@ [okulary.core :as l] [rumext.v2 :as mf])) -(defn- make-workspace-ready-ref - [file-id] - (l/derived (fn [state] - (and (= file-id (:workspace-ready state)) - (some? (dsh/lookup-file-data state file-id)))) - st/state)) - (mf/defc workspace-content* {::mf/private true} [{:keys [file layout page wglobal]}] @@ -138,22 +130,18 @@ key (events/listen globals/window "blur" focus-out)] (partial events/unlistenByKey key))) - (mf/with-effect [page-id] - (if (some? page-id) - (st/emit! (dw/initialize-page page-id)) - (st/emit! (dcm/go-to-workspace ::rt/replace true))) - + (mf/with-effect [file page-id] + (st/emit! (dw/initialize-page (:id file) page-id)) (fn [] - (when (some? page-id) - (st/emit! (dw/finalize-page page-id))))) + (when page-id + (st/emit! (dw/finalize-page (:id file) page-id))))) (if (some? page) [:> workspace-content* {:file file :page page :wglobal wglobal :layout layout}] - [:& workspace-loader*]))) - + [:> workspace-loader*]))) (def ^:private ref:file-without-data (l/derived (fn [file] @@ -181,10 +169,6 @@ read-only? (mf/deref refs/workspace-read-only?) read-only? (or read-only? (not (:can-edit permissions))) - ready* (mf/with-memo [file-id] - (make-workspace-ready-ref file-id)) - ready? (mf/deref ready*) - design-tokens? (features/use-feature "design-tokens/v1") background-color (:background-color wglobal)] @@ -207,6 +191,10 @@ (st/emit! ::dps/force-persist (dw/finalize-workspace file-id)))) + (mf/with-effect [file page-id] + (when-not page-id + (st/emit! (dcm/go-to-workspace :file-id file-id ::rt/replace true)))) + [:> (mf/provider ctx/current-project-id) {:value project-id} [:> (mf/provider ctx/current-file-id) {:value file-id} [:> (mf/provider ctx/current-page-id) {:value page-id} @@ -219,9 +207,10 @@ :touch-action "none"}} [:> context-menu*] - (if ^boolean ready? - [:> workspace-page* {:page-id page-id - :file file - :wglobal wglobal - :layout layout}] + (if (some? file) + [:> workspace-page* + {:page-id page-id + :file file + :wglobal wglobal + :layout layout}] [:> workspace-loader*])]]]]]]])) diff --git a/frontend/test/frontend_tests/logic/copying_and_duplicating_test.cljs b/frontend/test/frontend_tests/logic/copying_and_duplicating_test.cljs index 3d331c616..a6a59047a 100644 --- a/frontend/test/frontend_tests/logic/copying_and_duplicating_test.cljs +++ b/frontend/test/frontend_tests/logic/copying_and_duplicating_test.cljs @@ -63,10 +63,10 @@ target-container-id (or target-container-id (:parent-id shape))] (filter some? - [(when target-page-id (dw/initialize-page target-page-id)) + [(when target-page-id (dw/initialize-page (:id file) target-page-id)) (dws/select-shape target-container-id) (dw/paste-shapes pdata) - (when target-page-id (dw/initialize-page (:id page)))]))) + (when target-page-id (dw/initialize-page (:id file) (:id page)))]))) (defn- sync-file [file] (map (fn [component-tag]