diff --git a/.serena/memories/common/data-model-change-checklist.md b/.serena/memories/common/data-model-change-checklist.md index 91f00d1e6e..f4540f9ea3 100644 --- a/.serena/memories/common/data-model-change-checklist.md +++ b/.serena/memories/common/data-model-change-checklist.md @@ -6,6 +6,7 @@ - Do not treat nil as a distinct persisted state from absence. Import/export and cleanup paths may filter nil attrs away. - Avoid Clojure-special naming in exported object attrs, especially boolean names ending in `?`; exported/imported data must survive JSON/SVG/Transit and external tooling. - Any new shape attr that participates in component sync must be listed in `app.common.types.component/sync-attrs` with the correct touched group. Attrs absent from `sync-attrs` are ignored by component synchronization. +- Shape `:svg-attrs` is the one map whose keys are camelCase React prop names (`fillRule`, not `fill-rule`), because the SVG import path runs them through `app.common.svg/attrs->props`. The `.penpot` reader (`app.common.json/read-kebab-key`) rewrites every json key of every entry to kebab-case, nested maps included, so any code that decodes a binary file must run `:svg-attrs` back through `attrs->props` or the names are lost. `:svg-defs` and the `:content` tree of `svg-raw` shapes keep their source names and round-trip on their own. ## Cross-module update checklist diff --git a/backend/src/app/binfile/cleaner.clj b/backend/src/app/binfile/cleaner.clj index 66964b5358..035c0453cf 100644 --- a/backend/src/app/binfile/cleaner.clj +++ b/backend/src/app/binfile/cleaner.clj @@ -9,6 +9,7 @@ for recently imported shapes." (:require [app.common.data :as d] + [app.common.svg :as csvg] [app.common.types.shape :as cts] [app.common.uuid :as uuid])) @@ -105,6 +106,15 @@ :reverse-column :column-reverse dir)))) +(defn- fix-svg-attrs + "The json reader of the binfile rewrites every key of every entry to + kebab-case, but `:svg-attrs` keys are react prop names and are stored + in camelCase. `attrs->props` is the transform the svg import path + already applies to them, so running it again restores the names; it + is idempotent, so shapes that come in correct are left untouched." + [shape] + (d/update-when shape :svg-attrs csvg/attrs->props)) + (defn clean-shape-post-decode "A shape procesor that expected to be executed after schema decoding process but before validation." @@ -112,7 +122,8 @@ (-> shape (fix-shape-shadow-color) (fix-root-shape) - (fix-legacy-flex-dir))) + (fix-legacy-flex-dir) + (fix-svg-attrs))) (defn- fix-container [container] diff --git a/backend/test/backend_tests/binfile_test.clj b/backend/test/backend_tests/binfile_test.clj index a57624fa00..42edaaf4b5 100644 --- a/backend/test/backend_tests/binfile_test.clj +++ b/backend/test/backend_tests/binfile_test.clj @@ -24,6 +24,7 @@ [app.storage.tmp :as tmp] [backend-tests.helpers :as th] [backend-tests.storage-test :as stt] + [clojure.java.io :as jio] [clojure.test :as t] [cuerdas.core :as str] [datoteka.fs :as fs] @@ -188,6 +189,57 @@ ;; of failing with :child-not-found on the next update-file. (t/is (nil? (cfv/validate-file imported [])))))) +(defn- import-svg-attrs-asset + "Imports the `svg-attrs-camel-case.penpot` asset, a real penpot export + whose shapes carry `:svg-attrs` keys in camelCase (the format the + binary export writes), and returns the imported file." + [profile] + (let [input (-> "backend_tests/test_files/svg-attrs-camel-case.penpot" + io/resource + jio/file) + result (-> th/*system* + (assoc ::bfc/project-id (:default-project-id profile)) + (assoc ::bfc/profile-id (:id profile)) + (assoc ::bfc/input input) + (v3/import-files!))] + (bfc/get-file th/*system* (first result)))) + +(t/deftest import-binfile-v3-preserves-camel-case-svg-attrs + ;; The json reader used by the v3 import rewrites every key of every + ;; zip entry to kebab-case, and `:svg-attrs` is the one shape map + ;; whose keys are camelCase react prop names. A shape exported with + ;; `fillRule: "evenodd"` must not come back as `:fill-rule`, or the + ;; renderer falls back to the default fill rule and the shape is + ;; painted without its hole. + (let [profile (th/create-profile* 1) + file (import-svg-attrs-asset profile) + shape (get-in file [:data :pages-index + (uuid/uuid "fc80ab5f-1bf2-817c-8008-b5408f039100") + :objects + (uuid/uuid "ce3641bd-48c8-804c-8008-b54d35cc6f80")])] + + (t/is (= {:fillRule "evenodd"} + (:svg-attrs shape))))) + +(t/deftest import-binfile-v3-preserves-camel-case-svg-attrs-on-components + ;; Same guarantee for shapes stored inside a component: the v3 import + ;; cleans those in a different code path than page shapes. + (let [profile (th/create-profile* 1) + file (import-svg-attrs-asset profile) + shape (fn [component-id shape-id] + (-> file + (get-in [:data :components (uuid/uuid component-id) :objects]) + (get (uuid/uuid shape-id)) + :svg-attrs))] + + (t/is (= {:fillRule "evenodd"} + (shape "fae4bc76-0cc2-8057-8008-b540912cdf78" + "fae4bc76-0cc2-8057-8008-b540912774cb"))) + + (t/is (= {:fillRule "nonzero"} + (shape "fae4bc76-0cc2-8057-8008-b540912c917b" + "fae4bc76-0cc2-8057-8008-b540912774c8"))))) + (t/deftest export-binfile-v3 (let [profile (th/create-profile* 1) file (prepare-simple-file profile) diff --git a/backend/test/backend_tests/test_files/svg-attrs-camel-case.penpot b/backend/test/backend_tests/test_files/svg-attrs-camel-case.penpot new file mode 100644 index 0000000000..dd9513c343 Binary files /dev/null and b/backend/test/backend_tests/test_files/svg-attrs-camel-case.penpot differ