mirror of
https://github.com/penpot/penpot.git
synced 2026-03-26 13:20:51 +01:00
🐛 Fix pluings API theme.addSet() crash caused by async state race in token-set proxy (#8700)
When `catalog.addSet()` creates a new token set, `st/emit!` is async —
the set is not yet in `@st/state` when the returned proxy is used.
Calling `theme.addSet(proxy)` immediately after reads `.name` from the
proxy, which calls `locate-token-set` on stale state → returns nil →
`enable-set` conjs nil into the theme's `:sets` → backend rejects with
400 (`:sets #{nil}`) → workspace reloads → plugin disconnects.
Fix: store `initial-name` in the proxy at construction time as a
fallback for the `:name` getter during the async propagation window.
Also add nil guards in `addSet`/`removeSet` as defense-in-depth.
Closes #8698
Signed-off-by: rodo <roland@dolltons.com>
This commit is contained in:
Roland
@@ -203,123 +203,130 @@
|
||||
(obj/type-of? p "TokenSetProxy"))
|
||||
|
||||
(defn token-set-proxy
|
||||
[plugin-id file-id id]
|
||||
(obj/reify {:name "TokenSetProxy"
|
||||
:on-error (u/handle-error plugin-id)}
|
||||
:$plugin {:enumerable false :get (constantly plugin-id)}
|
||||
:$file-id {:enumerable false :get (constantly file-id)}
|
||||
:$id {:enumerable false :get (constantly id)}
|
||||
([plugin-id file-id id]
|
||||
(token-set-proxy plugin-id file-id id nil))
|
||||
([plugin-id file-id id initial-name]
|
||||
(obj/reify {:name "TokenSetProxy"
|
||||
:on-error (u/handle-error plugin-id)}
|
||||
:$plugin {:enumerable false :get (constantly plugin-id)}
|
||||
:$file-id {:enumerable false :get (constantly file-id)}
|
||||
:$id {:enumerable false :get (constantly id)}
|
||||
|
||||
:id
|
||||
{:get #(dm/str id)}
|
||||
:id
|
||||
{:get #(dm/str id)}
|
||||
|
||||
:name
|
||||
{:this true
|
||||
:get
|
||||
:name
|
||||
{:this true
|
||||
:get
|
||||
(fn [_]
|
||||
;; Prefer the authoritative state lookup; fall back to initial-name
|
||||
;; when the async state update from `catalog.addSet()` hasn't
|
||||
;; propagated yet.
|
||||
(let [set (u/locate-token-set file-id id)]
|
||||
(if (some? set)
|
||||
(ctob/get-name set)
|
||||
initial-name)))
|
||||
:schema (cfo/make-token-set-name-schema
|
||||
(u/locate-tokens-lib file-id)
|
||||
id)
|
||||
:set
|
||||
(fn [_ name]
|
||||
(let [set (u/locate-token-set file-id id)]
|
||||
(st/emit! (dwtl/rename-token-set set name))))}
|
||||
|
||||
:active
|
||||
{:this true
|
||||
:enumerable false
|
||||
:get
|
||||
(fn [_]
|
||||
(let [tokens-lib (u/locate-tokens-lib file-id)
|
||||
set (u/locate-token-set file-id id)]
|
||||
(ctob/token-set-active? tokens-lib (ctob/get-name set))))
|
||||
:schema ::sm/boolean
|
||||
:set
|
||||
(fn [_ value]
|
||||
(let [set (u/locate-token-set file-id id)]
|
||||
(st/emit! (dwtl/set-enabled-token-set (ctob/get-name set) value))))}
|
||||
|
||||
:toggleActive
|
||||
(fn [_]
|
||||
(let [set (u/locate-token-set file-id id)]
|
||||
(ctob/get-name set)))
|
||||
:schema (cfo/make-token-set-name-schema
|
||||
(u/locate-tokens-lib file-id)
|
||||
id)
|
||||
:set
|
||||
(fn [_ name]
|
||||
(let [set (u/locate-token-set file-id id)]
|
||||
(st/emit! (dwtl/rename-token-set set name))))}
|
||||
(st/emit! (dwtl/toggle-token-set (ctob/get-name set)))))
|
||||
|
||||
:active
|
||||
{:this true
|
||||
:enumerable false
|
||||
:get
|
||||
(fn [_]
|
||||
(let [tokens-lib (u/locate-tokens-lib file-id)
|
||||
set (u/locate-token-set file-id id)]
|
||||
(ctob/token-set-active? tokens-lib (ctob/get-name set))))
|
||||
:schema ::sm/boolean
|
||||
:set
|
||||
(fn [_ value]
|
||||
(let [set (u/locate-token-set file-id id)]
|
||||
(st/emit! (dwtl/set-enabled-token-set (ctob/get-name set) value))))}
|
||||
:tokens
|
||||
{:this true
|
||||
:enumerable false
|
||||
:get
|
||||
(fn [_]
|
||||
(let [tokens-lib (u/locate-tokens-lib file-id)]
|
||||
(->> (ctob/get-tokens tokens-lib id)
|
||||
(vals)
|
||||
(map #(token-proxy plugin-id file-id id (:id %)))
|
||||
(apply array))))}
|
||||
|
||||
:toggleActive
|
||||
(fn [_]
|
||||
(let [set (u/locate-token-set file-id id)]
|
||||
(st/emit! (dwtl/toggle-token-set (ctob/get-name set)))))
|
||||
:tokensByType
|
||||
{:this true
|
||||
:enumerable false
|
||||
:get
|
||||
(fn [_]
|
||||
(let [tokens-lib (u/locate-tokens-lib file-id)
|
||||
tokens (ctob/get-tokens tokens-lib id)]
|
||||
(->> tokens
|
||||
(vals)
|
||||
(sort-by :name)
|
||||
(group-by #(cto/token-type->dtcg-token-type (:type %)))
|
||||
(into [])
|
||||
(mapv (fn [[type tokens]]
|
||||
#js [(name type)
|
||||
(->> tokens
|
||||
(map #(token-proxy plugin-id file-id id (:id %)))
|
||||
(apply array))]))
|
||||
(apply array))))}
|
||||
|
||||
:tokens
|
||||
{:this true
|
||||
:enumerable false
|
||||
:get
|
||||
(fn [_]
|
||||
(let [tokens-lib (u/locate-tokens-lib file-id)]
|
||||
(->> (ctob/get-tokens tokens-lib id)
|
||||
(vals)
|
||||
(map #(token-proxy plugin-id file-id id (:id %)))
|
||||
(apply array))))}
|
||||
:getTokenById
|
||||
{:enumerable false
|
||||
:schema [:tuple ::sm/uuid]
|
||||
:fn (fn [token-id]
|
||||
(let [token (u/locate-token file-id id token-id)]
|
||||
(when (some? token)
|
||||
(token-proxy plugin-id file-id id token-id))))}
|
||||
|
||||
:tokensByType
|
||||
{:this true
|
||||
:enumerable false
|
||||
:get
|
||||
(fn [_]
|
||||
(let [tokens-lib (u/locate-tokens-lib file-id)
|
||||
tokens (ctob/get-tokens tokens-lib id)]
|
||||
(->> tokens
|
||||
(vals)
|
||||
(sort-by :name)
|
||||
(group-by #(cto/token-type->dtcg-token-type (:type %)))
|
||||
(into [])
|
||||
(mapv (fn [[type tokens]]
|
||||
#js [(name type)
|
||||
(->> tokens
|
||||
(map #(token-proxy plugin-id file-id id (:id %)))
|
||||
(apply array))]))
|
||||
(apply array))))}
|
||||
:addToken
|
||||
{:enumerable false
|
||||
:schema (fn [args]
|
||||
[:tuple (-> (cfo/make-token-schema
|
||||
(-> (u/locate-tokens-lib file-id) (ctob/get-tokens id))
|
||||
(cto/dtcg-token-type->token-type (-> args (first) (get "type"))))
|
||||
;; Don't allow plugins to set the id
|
||||
(sm/dissoc-key :id)
|
||||
;; Instruct the json decoder in obj/reify not to process map keys (:key-fn below)
|
||||
;; and set a converter that changes DTCG types to internal types (:decode/json).
|
||||
;; E.g. "FontFamilies" -> :font-family or "BorderWidth" -> :stroke-width
|
||||
(sm/update-properties assoc :decode/json cfo/convert-dtcg-token))])
|
||||
:decode/options {:key-fn identity}
|
||||
:fn (fn [attrs]
|
||||
(let [tokens-lib (u/locate-tokens-lib file-id)
|
||||
token (ctob/make-token attrs)
|
||||
tokens-tree (-> (ctob/get-tokens-in-active-sets tokens-lib)
|
||||
(assoc (:name token) token))
|
||||
resolved-tokens (ts/resolve-tokens tokens-tree)
|
||||
|
||||
:getTokenById
|
||||
{:enumerable false
|
||||
:schema [:tuple ::sm/uuid]
|
||||
:fn (fn [token-id]
|
||||
(let [token (u/locate-token file-id id token-id)]
|
||||
(when (some? token)
|
||||
(token-proxy plugin-id file-id id token-id))))}
|
||||
{:keys [errors resolved-value] :as resolved-token}
|
||||
(get resolved-tokens (:name token))]
|
||||
|
||||
:addToken
|
||||
{:enumerable false
|
||||
:schema (fn [args]
|
||||
[:tuple (-> (cfo/make-token-schema
|
||||
(-> (u/locate-tokens-lib file-id) (ctob/get-tokens id))
|
||||
(cto/dtcg-token-type->token-type (-> args (first) (get "type"))))
|
||||
;; Don't allow plugins to set the id
|
||||
(sm/dissoc-key :id)
|
||||
;; Instruct the json decoder in obj/reify not to process map keys (:key-fn below)
|
||||
;; and set a converter that changes DTCG types to internal types (:decode/json).
|
||||
;; E.g. "FontFamilies" -> :font-family or "BorderWidth" -> :stroke-width
|
||||
(sm/update-properties assoc :decode/json cfo/convert-dtcg-token))])
|
||||
:decode/options {:key-fn identity}
|
||||
:fn (fn [attrs]
|
||||
(let [tokens-lib (u/locate-tokens-lib file-id)
|
||||
token (ctob/make-token attrs)
|
||||
tokens-tree (-> (ctob/get-tokens-in-active-sets tokens-lib)
|
||||
(assoc (:name token) token))
|
||||
resolved-tokens (ts/resolve-tokens tokens-tree)
|
||||
(if resolved-value
|
||||
(do (st/emit! (dwtl/create-token id token))
|
||||
(token-proxy plugin-id file-id id (:id token)))
|
||||
(do (u/not-valid plugin-id :addToken (str errors))
|
||||
nil))))}
|
||||
|
||||
{:keys [errors resolved-value] :as resolved-token}
|
||||
(get resolved-tokens (:name token))]
|
||||
:duplicate
|
||||
(fn []
|
||||
(st/emit! (dwtl/duplicate-token-set id)))
|
||||
|
||||
(if resolved-value
|
||||
(do (st/emit! (dwtl/create-token id token))
|
||||
(token-proxy plugin-id file-id id (:id token)))
|
||||
(do (u/not-valid plugin-id :addToken (str errors))
|
||||
nil))))}
|
||||
|
||||
:duplicate
|
||||
(fn []
|
||||
(st/emit! (dwtl/duplicate-token-set id)))
|
||||
|
||||
:remove
|
||||
(fn []
|
||||
(st/emit! (dwtl/delete-token-set id)))))
|
||||
:remove
|
||||
(fn []
|
||||
(st/emit! (dwtl/delete-token-set id))))))
|
||||
|
||||
(defn token-theme-proxy? [p]
|
||||
(obj/type-of? p "TokenThemeProxy"))
|
||||
@@ -408,15 +415,26 @@
|
||||
{:enumerable false
|
||||
:schema [:tuple [:fn token-set-proxy?]]
|
||||
:fn (fn [token-set]
|
||||
(let [theme (u/locate-token-theme file-id id)]
|
||||
(st/emit! (dwtl/update-token-theme id (ctob/enable-set theme (obj/get token-set :name))))))}
|
||||
;; Resolve the set name before the theme lookup. The proxy's :name
|
||||
;; getter now falls back to `initial-name` when state hasn't
|
||||
;; propagated, so this is safe even for freshly created sets.
|
||||
;; Guard against nil to prevent `enable-set` from conj'ing nil
|
||||
;; into the theme's :sets — which would send `:sets #{nil}` to the
|
||||
;; backend and crash the workspace.
|
||||
(let [set-name (obj/get token-set :name)
|
||||
theme (u/locate-token-theme file-id id)]
|
||||
(when (and (some? set-name) (some? theme))
|
||||
(st/emit! (dwtl/update-token-theme id (ctob/enable-set theme set-name))))))}
|
||||
|
||||
:removeSet
|
||||
{:enumerable false
|
||||
:schema [:tuple [:fn token-set-proxy?]]
|
||||
:fn (fn [token-set]
|
||||
(let [theme (u/locate-token-theme file-id id)]
|
||||
(st/emit! (dwtl/update-token-theme id (ctob/disable-set theme (obj/get token-set :name))))))}
|
||||
;; Same nil guard as addSet — see comment above.
|
||||
(let [set-name (obj/get token-set :name)
|
||||
theme (u/locate-token-theme file-id id)]
|
||||
(when (and (some? set-name) (some? theme))
|
||||
(st/emit! (dwtl/update-token-theme id (ctob/disable-set theme set-name))))))}
|
||||
|
||||
:duplicate
|
||||
(fn []
|
||||
@@ -484,7 +502,10 @@
|
||||
(let [attrs (update attrs :name ctob/normalize-set-name)
|
||||
set (ctob/make-token-set attrs)]
|
||||
(st/emit! (dwtl/create-token-set set))
|
||||
(token-set-proxy plugin-id file-id (ctob/get-id set))))}
|
||||
;; Pass the set name as `initial-name` so the proxy can resolve
|
||||
;; it immediately, before the async `st/emit!` above propagates
|
||||
;; the new set into `@st/state`.
|
||||
(token-set-proxy plugin-id file-id (ctob/get-id set) (ctob/get-name set))))}
|
||||
|
||||
:getThemeById
|
||||
{:enumerable false
|
||||
|
||||
Reference in New Issue
Block a user