🐛 Fix plugin API fills/strokes arrays read-only (#9161)

* 🐛 Fix plugin API fills/strokes arrays read-only Signed-off-by: RenzoMXD <170978465+RenzoMXD@users.noreply.github.com> * 🐛 Support mutable plugin fill and stroke gradients --------- Signed-off-by: RenzoMXD <170978465+RenzoMXD@users.noreply.github.com> Signed-off-by: Andrey Antukh <niwi@niwi.nz> Co-authored-by: Andrey Antukh <niwi@niwi.nz>
2026-07-01 20:05:26 +00:00 · 2026-05-07 13:10:48 +02:00 · 2026-05-07 13:10:48 +02:00 · 691679d90b
commit 691679d90b
parent bd91036b95
4 changed files with 240 additions and 47 deletions
--- a/CHANGES.md
+++ b/CHANGES.md
@ -133,6 +133,7 @@
 - Fix broken update library notification link in the UI [Github #9070](https://github.com/penpot/penpot/issues/9070)
 - Fix plugin API `ShapeBase.component()` returning the outermost component instead of the immediate component in case of nested component instances [Github #9183](https://github.com/penpot/penpot/issues/9183)
 - Fix missing `labels.open` translation that surfaced the raw key as the typography font open-button `aria-label`, breaking screen-reader output (by @MilosM348)
+- Fix plugin API `shape.fills` and `shape.strokes` arrays being read-only [Github #8357](https://github.com/penpot/penpot/issues/8357)


 ## 2.15.0 (Unreleased)
--- a/frontend/src/app/plugins/format.cljs
+++ b/frontend/src/app/plugins/format.cljs
@ -26,6 +26,97 @@
  (when (some? coll)
    (apply array (keep format-fn coll))))

+(defn- numeric-index?
+  [prop]
+  (and (string? prop) (boolean (re-matches #"\d+" prop))))
+
+(defn- normalize-exclusive-color-props!
+  [target prop]
+  (case prop
+    "fillColor"
+    (do
+      (js-delete target "fillColorGradient")
+      (js-delete target "fillImage"))
+
+    "fillColorGradient"
+    (do
+      (js-delete target "fillColor")
+      (js-delete target "fillImage"))
+
+    "fillImage"
+    (do
+      (js-delete target "fillColor")
+      (js-delete target "fillColorGradient"))
+
+    "strokeColor"
+    (js-delete target "strokeColorGradient")
+
+    "strokeColorGradient"
+    (js-delete target "strokeColor")
+
+    nil))
+
+(declare wrap-mutable-value)
+
+(defn- wrap-mutable-object
+  [^js js-obj commit!]
+  (doseq [prop (js/Object.keys js-obj)]
+    (obj/set! js-obj prop (wrap-mutable-value (obj/get js-obj prop) commit!)))
+  (js/Proxy. js-obj
+             #js {:set (fn [target prop value]
+                         (obj/set! target prop (wrap-mutable-value value commit!))
+                         (normalize-exclusive-color-props! target prop)
+                         (commit!)
+                         true)
+                  :deleteProperty (fn [target prop]
+                                    (js-delete target prop)
+                                    (commit!)
+                                    true)}))
+
+(defn- wrap-mutable-array
+  [^js js-arr commit!]
+  (doseq [index (range (.-length js-arr))]
+    (obj/set! js-arr index (wrap-mutable-value (obj/get js-arr index) commit!)))
+  (js/Proxy. js-arr
+             #js {:set (fn [target prop value]
+                         (if (or (numeric-index? prop) (= prop "length"))
+                           (do
+                             (if (numeric-index? prop)
+                               (obj/set! target prop (wrap-mutable-value value commit!))
+                               (obj/set! target prop value))
+                             (commit!)
+                             true)
+                           false))
+                  :deleteProperty (fn [target prop]
+                                    (if (numeric-index? prop)
+                                      (do
+                                        (js-delete target prop)
+                                        true)
+                                      false))}))
+
+(defn- wrap-mutable-value
+  [value commit!]
+  (cond
+    (obj/array? value)
+    (wrap-mutable-array value commit!)
+
+    (obj/plain-object? value)
+    (wrap-mutable-object value commit!)
+
+    :else
+    value))
+
+(defn wrap-mutable-element
+  [^js js-obj commit!]
+  (when (some? js-obj)
+    (wrap-mutable-value js-obj commit!)))
+
+(defn mutable-proxy-array
+  [coll format-fn commit-fn]
+  (let [raw-arr (format-array format-fn coll)
+        commit! (fn [] (commit-fn raw-arr))]
+    (wrap-mutable-array raw-arr commit!)))
+
 (defn format-mixed
  [value]
  (if (= value :multiple)
@ -198,16 +289,17 @@
          :fillImage (format-image fill-image)})))

 (defn format-fills
-  [fills]
-  (cond
-    (= fills :multiple)
-    "mixed"
+  ([fills] (format-fills fills nil))
+  ([fills commit-fn]
+   (cond
+     (= fills :multiple) "mixed"
+     (= fills "mixed")   "mixed"

-    (= fills "mixed")
-    "mixed"
+     (and (some? fills) (fn? commit-fn))
+     (mutable-proxy-array fills format-fill commit-fn)

-    (some? fills)
-    (format-array format-fill fills)))
+     :else
+     (format-array format-fill fills))))

 ;; export interface Stroke {
 ;;   strokeColor?: string;
@ -240,9 +332,11 @@
          :strokeColorGradient (format-gradient stroke-color-gradient)})))

 (defn format-strokes
-  [strokes]
-  (when (some? strokes)
-    (format-array format-stroke strokes)))
+  ([strokes] (format-strokes strokes nil))
+  ([strokes commit-fn]
+   (if (and (some? strokes) (fn? commit-fn))
+     (mutable-proxy-array strokes format-stroke commit-fn)
+     (format-array format-stroke strokes))))

 ;; export interface Blur {
 ;;   id?: string;
--- a/frontend/src/app/plugins/shape.cljs
+++ b/frontend/src/app/plugins/shape.cljs
@ -173,6 +173,38 @@
    :hidden false}
   blur))

+(defn commit-fills!
+  [plugin-id ^js self value]
+  (let [shape (u/proxy->shape self)
+        id    (:id shape)
+        value (parser/parse-fills value)]
+    (cond
+      (not (sm/validate [:vector types.fills/schema:fill] value))
+      (u/not-valid plugin-id :fills value)
+
+      (cfh/text-shape? shape)
+      (st/emit! (dwt/update-attrs id {:fills value}))
+
+      (not (r/check-permission plugin-id "content:write"))
+      (u/not-valid plugin-id :fills "Plugin doesn't have 'content:write' permission")
+
+      :else
+      (st/emit! (dwsh/update-shapes [id] #(assoc % :fills value))))))
+
+(defn commit-strokes!
+  [plugin-id ^js self value]
+  (let [id    (obj/get self "$id")
+        value (parser/parse-strokes value)]
+    (cond
+      (not (sm/validate [:vector cts/schema:stroke] value))
+      (u/not-valid plugin-id :strokes value)
+
+      (not (r/check-permission plugin-id "content:write"))
+      (u/not-valid plugin-id :strokes "Plugin doesn't have 'content:write' permission")
+
+      :else
+      (st/emit! (dwsh/update-shapes [id] #(assoc % :strokes value))))))
+
 (defn shape-proxy? [p]
  (obj/type-of? p "ShapeProxy"))

@ -726,43 +758,19 @@
           ;; Strokes and fills
           :fills
           {:this true
-            :get #(if (cfh/text-shape? data)
-                    (-> % u/proxy->shape text-props :fills format/format-fills)
-                    (-> % u/proxy->shape :fills format/format-fills))
-            :set
-            (fn [self value]
-              (let [shape (u/proxy->shape self)
-                    id    (:id shape)
-                    value (parser/parse-fills value)]
-                (cond
-                  (not (sm/validate [:vector types.fills/schema:fill] value))
-                  (u/not-valid plugin-id :fills value)
-
-                  (cfh/text-shape? shape)
-                  (st/emit! (dwt/update-attrs id {:fills value}))
-
-                  (not (r/check-permission plugin-id "content:write"))
-                  (u/not-valid plugin-id :fills "Plugin doesn't have 'content:write' permission")
-
-                  :else
-                  (st/emit! (dwsh/update-shapes [id] #(assoc % :fills value))))))}
+            :get (fn [^js self]
+                   (let [fills (if (cfh/text-shape? data)
+                                 (-> self u/proxy->shape text-props :fills)
+                                 (-> self u/proxy->shape :fills))]
+                     (format/format-fills fills #(commit-fills! plugin-id self %))))
+            :set (fn [self value] (commit-fills! plugin-id self value))}

           :strokes
           {:this true
-            :get #(-> % u/proxy->shape :strokes format/format-strokes)
-            :set
-            (fn [self value]
-              (let [id (obj/get self "$id")
-                    value (parser/parse-strokes value)]
-                (cond
-                  (not (sm/validate [:vector cts/schema:stroke] value))
-                  (u/not-valid plugin-id :strokes value)
-
-                  (not (r/check-permission plugin-id "content:write"))
-                  (u/not-valid plugin-id :strokes "Plugin doesn't have 'content:write' permission")
-
-                  :else
-                  (st/emit! (dwsh/update-shapes [id] #(assoc % :strokes value))))))}
+            :get (fn [^js self]
+                   (format/format-strokes (-> self u/proxy->shape :strokes)
+                                          #(commit-strokes! plugin-id self %)))
+            :set (fn [self value] (commit-strokes! plugin-id self value))}

           :layoutChild
           {:this true
--- a/frontend/test/frontend_tests/plugins/context_shapes_test.cljs
+++ b/frontend/test/frontend_tests/plugins/context_shapes_test.cljs
@ -11,6 +11,7 @@
   [app.common.uuid :as uuid]
   [app.main.store :as st]
   [app.plugins.api :as api]
+   [app.util.object :as obj]
   [cljs.test :as t :include-macros true]
   [frontend-tests.helpers.state :as ths]
   [frontend-tests.helpers.wasm :as thw]))
@ -30,7 +31,28 @@
            ^js shape   (.createRectangle context)

            get-shape-path
-            #(vector :files (aget file "$id") :data :pages-index (aget page "$id") :objects (aget shape "$id") %)]
+            #(vector :files (aget file "$id") :data :pages-index (aget page "$id") :objects (aget shape "$id") %)
+
+            gradient
+            (fn []
+              #js {:type "linear"
+                   :startX 0.5
+                   :startY 0
+                   :endX 0.5
+                   :endY 1
+                   :width 1
+                   :stops #js [#js {:color "#b400ff" :opacity 1 :offset 0}
+                               #js {:color "#0c3fd5" :opacity 1 :offset 1}]})
+
+            parsed-gradient
+            {:type :linear
+             :start-x 0.5
+             :start-y 0
+             :end-x 0.5
+             :end-y 1
+             :width 1
+             :stops [{:color "#b400ff" :opacity 1 :offset 0}
+                     {:color "#0c3fd5" :opacity 1 :offset 1}]}]

        (t/testing "Basic shape properites"
          (t/testing " - name"
@ -218,7 +240,75 @@
            (t/is (= (get-in @store (get-shape-path :strokes)) [{:stroke-color "#fabada" :stroke-opacity 1 :stroke-width 5}]))
            (t/is (= (-> (. ^js shape -strokes) (aget 0) (aget "strokeColor")) "#fabada"))
            (t/is (= (-> (. ^js shape -strokes) (aget 0) (aget "strokeOpacity")) 1))
-            (t/is (= (-> (. ^js shape -strokes) (aget 0) (aget "strokeWidth")) 5))))
+            (t/is (= (-> (. ^js shape -strokes) (aget 0) (aget "strokeWidth")) 5)))
+
+          (t/testing " - fills per-element property mutation (bug #8357)"
+            (set! (.-fills shape) #js [#js {:fillColor "#fabada" :fillOpacity 1}])
+            (obj/set! (aget (.-fills shape) 0) "fillColor" "#ff0000")
+            (t/is (= (get-in @store (get-shape-path :fills)) [{:fill-color "#ff0000" :fill-opacity 1}]))
+            (t/is (= (-> (. shape -fills) (aget 0) (aget "fillColor")) "#ff0000")))
+
+          (t/testing " - fills element replacement (bug #8357)"
+            (set! (.-fills shape) #js [#js {:fillColor "#fabada" :fillOpacity 1}])
+            (aset (.-fills shape) 0 #js {:fillColor "#00ff00" :fillOpacity 0.5})
+            (t/is (= (get-in @store (get-shape-path :fills)) [{:fill-color "#00ff00" :fill-opacity 0.5}])))
+
+          (t/testing " - fills push/pop (bug #8357)"
+            (set! (.-fills shape) #js [#js {:fillColor "#fabada" :fillOpacity 1}])
+            (.push (.-fills shape) #js {:fillColor "#00ff00" :fillOpacity 1})
+            (t/is (= (get-in @store (get-shape-path :fills))
+                     [{:fill-color "#fabada" :fill-opacity 1}
+                      {:fill-color "#00ff00" :fill-opacity 1}]))
+            (.pop (.-fills shape))
+            (t/is (= (get-in @store (get-shape-path :fills)) [{:fill-color "#fabada" :fill-opacity 1}])))
+
+          (t/testing " - fills gradient assignment replaces solid color (bug #8357)"
+            (set! (.-fills shape) #js [#js {:fillColor "#fabada" :fillOpacity 1}])
+            (obj/set! (aget (.-fills shape) 0) "fillColorGradient" (gradient))
+            (t/is (= (get-in @store (get-shape-path :fills))
+                     [{:fill-opacity 1 :fill-color-gradient parsed-gradient}]))
+            (t/is (nil? (-> (. shape -fills) (aget 0) (aget "fillColor")))))
+
+          (t/testing " - fills nested gradient mutation (bug #8357)"
+            (set! (.-fills shape) #js [#js {:fillColorGradient (gradient) :fillOpacity 1}])
+            (let [fill-gradient (-> (. shape -fills) (aget 0) (aget "fillColorGradient"))
+                  stop          (-> fill-gradient (aget "stops") (aget 0))]
+              (obj/set! fill-gradient "startX" 0.25)
+              (obj/set! stop "color" "#ffffff")
+              (t/is (= (get-in @store (get-shape-path :fills))
+                       [{:fill-opacity 1
+                         :fill-color-gradient (-> parsed-gradient
+                                                  (assoc :start-x 0.25)
+                                                  (assoc-in [:stops 0 :color] "#ffffff"))}]))))
+
+          (t/testing " - strokes per-element property mutation (bug #8357)"
+            (set! (.-strokes shape) #js [#js {:strokeColor "#fabada" :strokeOpacity 1 :strokeWidth 5}])
+            (obj/set! (aget (.-strokes shape) 0) "strokeColor" "#0000ff")
+            (t/is (= (get-in @store (get-shape-path :strokes)) [{:stroke-color "#0000ff" :stroke-opacity 1 :stroke-width 5}])))
+
+          (t/testing " - strokes element replacement (bug #8357)"
+            (set! (.-strokes shape) #js [#js {:strokeColor "#fabada" :strokeOpacity 1 :strokeWidth 5}])
+            (aset (.-strokes shape) 0 #js {:strokeColor "#00ff00" :strokeOpacity 0.5 :strokeWidth 2})
+            (t/is (= (get-in @store (get-shape-path :strokes)) [{:stroke-color "#00ff00" :stroke-opacity 0.5 :stroke-width 2}])))
+
+          (t/testing " - strokes gradient assignment replaces solid color (bug #8357)"
+            (set! (.-strokes shape) #js [#js {:strokeColor "#fabada" :strokeOpacity 1 :strokeWidth 5}])
+            (obj/set! (aget (.-strokes shape) 0) "strokeColorGradient" (gradient))
+            (t/is (= (get-in @store (get-shape-path :strokes))
+                     [{:stroke-opacity 1 :stroke-width 5 :stroke-color-gradient parsed-gradient}])))
+
+          (t/testing " - strokes nested gradient mutation (bug #8357)"
+            (set! (.-strokes shape) #js [#js {:strokeColorGradient (gradient) :strokeOpacity 1 :strokeWidth 5}])
+            (let [stroke-gradient (-> (. shape -strokes) (aget 0) (aget "strokeColorGradient"))
+                  stop            (-> stroke-gradient (aget "stops") (aget 1))]
+              (obj/set! stroke-gradient "endY" 0.75)
+              (obj/set! stop "opacity" 0.25)
+              (t/is (= (get-in @store (get-shape-path :strokes))
+                       [{:stroke-opacity 1
+                         :stroke-width 5
+                         :stroke-color-gradient (-> parsed-gradient
+                                                    (assoc :end-y 0.75)
+                                                    (assoc-in [:stops 1 :opacity] 0.25))}])))))

        (t/testing "Relative properties"
          (let [board (.createBoard context)]