🐛 Keep camelCase svg attribute names when importing a penpot file (#11974)

The json reader of the binfile v3 import rewrites every key of every
entry to kebab-case, nested maps included. Shape `:svg-attrs` is the one
map whose keys are camelCase react prop names, because the svg import
path runs them through `attrs->props`, so an attribute exported as
`fillRule` came back as `:fill-rule` and was stored that way.

The renderer looks the attribute up by its camelCase name, does not find
it and falls back to the default fill rule, so a shape exported with
`fillRule: evenodd` was painted without its hole, and the attributes
panel showed `fill-rule`.

`clean-shape-post-decode` already runs on every shape right after the
schema decode, for page shapes and component objects alike, so the
repair goes there: run `:svg-attrs` back through `attrs->props`, the
same transform that built the keys. It is idempotent, so shapes that
arrive correct are left untouched.

The new tests import a real export that carries the attribute, for
page shapes and component shapes.

Closes #11954

AI-assisted-by: space-bunny-free
This commit is contained in:
Andrey Antukh 2026-09-29 14:15:21 +02:00 committed by GitHub
parent dc0ea3a69c
commit 0de865748d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 65 additions and 1 deletions

View File

@ -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

View File

@ -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]

View File

@ -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)