github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-09-08 13:09:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Fix create nil typography token (#11489)

Browse Source 
* 🐛 Fix stroke width token dropdown closing mid-interaction

The stroke-row* key included a hash of applied-tokens, forcing a full
remount whenever the async token-resolution pipeline updated that
state (e.g. after a detach action settles). If the remount landed
while the width dropdown was open, it destroyed the dropdown and
reset its open state, permanently closing it before the user could
pick a token.

Drop the hash from the key so the row only remounts on actual
structural changes (add/remove/reorder), not on every token
resolution tick.

AI-assisted-by: claude-sonnet-5

* 🐛 Fix crash when a typography token has a nil value

A composite typography token saved with no fields filled in stores a
nil :value. That value reached StyleDictionary's tokens-studio
font-styles preprocessor, which assumes a typography value is never
null and threw, crashing token resolution for every other token in
the file.

Reject the nil value at the source in the typography token form
validation, and as defense in depth, filter nil-valued tokens out of
every resolve-tokens* entry point before they reach StyleDictionary,
tagging them with the existing empty-input error instead.

Also remove a stray debug prn left in find-refs.

AI-assisted-by: claude-sonnet-5

* 🔧 Gitignore local CLAUDE.md

Keep the commit-conventions instructions file local to each
contributor's checkout instead of tracking it in the repo.

AI-assisted-by: claude-sonnet-5

* ⚡ Use a single transducer to tag invalid-value tokens

merge-invalid-value-tokens ran three separate passes over the token
map (remove, map, into) and then merged the result back in. Combine
the remove/map steps into one ns-level transducer, defined once
instead of rebuilt on every call, and pass resolved as the seed to
into so the trailing merge isn't needed either.

AI-assisted-by: claude-sonnet-5

* ♻️ Drop redundant t/testing wrapper in nil-value token test

The outer t/testing just repeated the deftest's own name and added
nothing the two inner t/testing blocks (each covering one concrete
assertion group) don't already say.

AI-assisted-by: claude-sonnet-5

* 🐛 Fail the nil-value token test on a resolution error

rx/sub! only handles the success case, so if token resolution ever
errors instead, done is never called and the async test hangs
instead of failing. Switch to rx/subs! with an error handler that
reports the failure and calls done, matching the pattern already
used elsewhere in the tokens test suite.

AI-assisted-by: claude-sonnet-5
This commit is contained in:
Eva Marco 2026-09-04 12:31:26 +02:00 committed by GitHub
parent fdcc4666e3
commit df48c834e3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 85 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
.gitignore vendored
View File

@ -97,6 +97,7 @@ opencode.json
/.idea
*.iml
/.claude
/CLAUDE.md
/.playwright-mcp
/.devenv/mcp/
/opencode.json

1
common/src/app/common/files/tokens.cljc
View File

@ -166,7 +166,6 @@
(not (ctob/token-name-path-exists? token-name tokens-tree)))
new-tokens))))]])
(defn find-refs [value]
(prn value)
(cond
(string? value)
(cto/find-token-value-references value)

50
frontend/src/app/main/data/style_dictionary.cljs
View File

@ -584,11 +584,38 @@
(into {}))]
(merge resolved dropped)))
(defn- valid-token-value?
[[_ token]]
(some? (:value token)))
(def ^:private xform-invalid-value-tokens
(comp
(remove valid-token-value?)
(map (fn [[k token]]
[k (assoc token :errors [(wte/get-error-code :error.token/empty-input)])]))))
(defn- merge-invalid-value-tokens
"Tokens with a `nil` value (e.g. a composite typography token saved with
no fields filled in) must never reach StyleDictionary: some of its
preprocessors (`@tokens-studio/sd-transforms`'s font-styles preprocessor,
in particular) assume a typography token's value is never null and throw
an uncaught exception when it is, taking down token resolution for the
whole file.
`tokens` is the full, unfiltered token map; `resolved` only contains the
valid subset that was actually sent to StyleDictionary. Tag the invalid
ones with the same \"empty value\" error the token forms already use
instead of ever letting them reach the resolver."
[tokens resolved]
(into resolved xform-invalid-value-tokens tokens))
(defn resolve-tokens
[tokens]
(let [tokens-tree (ctob/tokens-tree tokens)]
(->> (resolve-tokens-tree tokens-tree #(get tokens (sd-token-name %)))
(rx/map #(merge-name-collisions tokens %)))))
(let [valid-tokens (into {} (filter valid-token-value?) tokens)
tokens-tree (ctob/tokens-tree valid-tokens)]
(->> (resolve-tokens-tree tokens-tree #(get valid-tokens (sd-token-name %)))
(rx/map #(merge-name-collisions valid-tokens %))
(rx/map #(merge-invalid-value-tokens tokens %)))))
(defn resolve-tokens-interactive
"Interactive check of resolving tokens.
@ -610,15 +637,18 @@
computation we can restore any token, even clashing ones with the
same :name path by just looking up that :id in the ids map."
[tokens]
(let [{:keys [tokens-tree ids]} (ctob/backtrace-tokens-tree tokens)]
(->> (resolve-tokens-tree tokens-tree #(get ids (sd-token-uuid %)))
(rx/map #(merge-name-collisions tokens %)))))
(let [valid-tokens (into {} (filter valid-token-value?) tokens)
{:keys [tokens-tree ids]} (ctob/backtrace-tokens-tree valid-tokens)]
(->> (resolve-tokens-tree tokens-tree #(get ids (sd-token-uuid %)))
(rx/map #(merge-name-collisions valid-tokens %))
(rx/map #(merge-invalid-value-tokens tokens %)))))
(defn resolve-tokens-with-verbose-errors [tokens]
(resolve-tokens-tree
(ctob/tokens-tree tokens)
#(get tokens (sd-token-name %))
(StyleDictionary. (assoc default-config :log {:verbosity "verbose"}))))
(let [valid-tokens (into {} (filter valid-token-value?) tokens)]
(resolve-tokens-tree
(ctob/tokens-tree valid-tokens)
#(get valid-tokens (sd-token-name %))
(StyleDictionary. (assoc default-config :log {:verbosity "verbose"})))))
;; === Hooks

2
frontend/src/app/main/ui/workspace/sidebar/options/menus/stroke.cljs
View File

@ -276,7 +276,7 @@
(seq strokes)
[:> h/sortable-container* {}
(for [[index value] (d/enumerate (:strokes values []))]
[:> stroke-row* {:key (dm/str "stroke-" index "-" (hash applied-tokens))
[:> stroke-row* {:key (dm/str "stroke-" index)
:index index
:stroke value
:title (tr "workspace.options.stroke-color")

10
frontend/src/app/main/ui/workspace/tokens/management/forms/typography.cljs
View File

@ -18,7 +18,6 @@
[app.main.ui.workspace.tokens.management.forms.generic-form :as generic]
[app.main.ui.workspace.tokens.management.forms.validators :refer [check-coll-self-reference check-self-reference default-validate-token]]
[app.util.i18n :refer [tr]]
[beicon.v2.core :as rx]
[cuerdas.core :as str]
[rumext.v2 :as mf]))
@ -43,11 +42,14 @@
(defn- validate-typography-token
[{:keys [token-value] :as props}]
(cond
;; Entering form without a value - show no error just resolve nil
(nil? token-value) (rx/of nil)
;; Validate refrence string
(cto/composite-token-reference? token-value) (default-validate-token props)
;; Validate composite token
;; Validate composite token. `token-value` may be nil when the form is
;; submitted without any composite field filled in — normalize it to `{}`
;; so `check-empty-typography-token` catches it and rejects the submit,
;; instead of silently saving a token with a `nil` value (which later
;; crashes token resolution: the tokens-studio StyleDictionary
;; preprocessor assumes a typography token's value is never null).
:else
(-> props
(update :token-value

37
frontend/test/frontend_tests/tokens/style_dictionary_test.cljs
View File

@ -122,6 +122,43 @@
(get-in resolved-tokens ["typography.bad" :errors 0 :error/code])))
(done))))))))
;; Regression: a token with a `nil` value (e.g. a composite typography
;; token saved via the workspace form with no fields filled in) must never
;; reach StyleDictionary — its `tokens-studio` preprocessor assumes a
;; typography token's value is never null and throws an uncaught exception
;; on it, which used to take down resolution for every other token in the
;; file. It should be tagged with an empty-input error instead.
(t/deftest resolve-tokens-nil-value-test
(t/async
done
(let [tokens (-> (ctob/make-tokens-lib)
(ctob/add-set (ctob/make-token-set :id (cthi/new-id! :core-set)
:name "core"))
(ctob/add-token (cthi/id :core-set)
(ctob/make-token {:name "typography.empty"
:value nil
:type :typography}))
(ctob/add-token (cthi/id :core-set)
(ctob/make-token {:name "borderRadius.sm"
:value "12px"
:type :border-radius}))
(ctob/get-all-tokens-map))]
(->> (sd/resolve-tokens tokens)
(rx/subs!
(fn [resolved-tokens]
(t/testing "the nil-value token is tagged with an error instead of crashing"
(t/is (contains? resolved-tokens "typography.empty"))
(t/is (nil? (get-in resolved-tokens ["typography.empty" :resolved-value])))
(t/is (= :error.token/empty-input
(get-in resolved-tokens ["typography.empty" :errors 0 :error/code]))))
(t/testing "other tokens still resolve normally"
(t/is (= 12 (get-in resolved-tokens ["borderRadius.sm" :resolved-value])))))
(fn [err]
(t/do-report {:type :error :message "Stream error" :actual err})
(done))
(fn []
(done)))))))
(t/deftest resolve-tokens-interactive-test
(t/async
done