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

🔧 Revert migration for tokens with clashing names (#9950)

Browse Source 
* Revert "🐛 Detect duplicated token names in the whole library (#9034)"

This reverts commit 61cd7573553b1c5e9fc2d7300cf9b2c36b4dcbb6.

* 🔧 Preserve some enhancements and fixes that are still valid

* 🔧 Fix broken integration tests
This commit is contained in:
Andrés Moya 2026-06-02 09:09:58 +02:00 committed by GitHub
parent 53a4d2a18a
commit 06c9a18ab0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
19 changed files with 196 additions and 670 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
common/src/app/common/files/migrations.cljc
View File

@ -1805,13 +1805,6 @@
{})]
(cfcp/sync-component-id-with-ref-shape data libraries)))
(defmethod migrate-data "0021-repair-bad-tokens"
[data _]
(d/update-when data :tokens-lib
#(-> %
(ctob/fix-conflicting-token-names)
(ctob/fix-missing-sets-in-themes))))
(defmethod migrate-data "0021-fix-shape-svg-attrs"
[data _]
(some-> cfeat/*new* (swap! conj "fdata/shape-data-type"))
@ -1840,6 +1833,10 @@
(cfcp/fix-missing-swap-slots libraries)
(cfcp/sync-component-id-with-ref-shape libraries))))
(defmethod migrate-data "0023-repair-token-themes-with-inexistent-sets"
[data _]
(d/update-when data :tokens-lib ctob/fix-missing-sets-in-themes))
(def available-migrations
(into (d/ordered-set)
["legacy-2"
@ -1918,6 +1915,6 @@
"0018-remove-unneeded-objects-from-components"
"0019-fix-missing-swap-slots"
"0020-sync-component-id-with-near-main"
"0021-repair-bad-tokens"
"0021-fix-shape-svg-attrs"
"0022-normalize-component-root-and-resync"]))
"0022-normalize-component-root-and-resync"
"0023-repair-token-themes-with-inexistent-sets"]))

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

@ -134,26 +134,24 @@
(defn make-token-name-schema
"Dynamically generates a schema to check a token name, adding translated error messages
and additional validations:
and two additional validations:
- Min and max length.
- Checks if other token with a path derived from the name already exists in the library.
e.g. it's not allowed to create a token `foo.bar` if a token `foo` already exists.
- Also checks if there is a token with the exact same name in the current set, but different
from the current token."
[tokens-lib set-id token-id]
- Checks if other token with a path derived from the name already exists at `tokens-tree`.
e.g. it's not allowed to create a token `foo.bar` if a token `foo` already exists."
[tokens-tree]
[:and
[:string {:min 1 :max 255 :error/fn #(str (:value %) (tr "workspace.tokens.token-name-length-validation-error"))}]
(-> cto/schema:token-name
(sm/update-properties assoc :error/fn #(str (:value %) (tr "workspace.tokens.token-name-validation-error"))))
[:fn {:error/fn #(tr "workspace.tokens.token-name-duplication-validation-error" (:value %))}
#(or (nil? tokens-lib)
(not (ctob/token-name-path-exists? % tokens-lib set-id token-id)))]])
#(and (some? tokens-tree)
(not (ctob/token-name-path-exists? % tokens-tree)))]])
(defn make-node-token-name-schema
"Dynamically generates a schema to check the name of a token node, that may be a final token or a group.
This runs same checks as make-token-name-schema, but for all tokens that will be renamed by this change,
if the group already contains tokens."
[active-tokens tokens-lib node set-id]
[active-tokens tokens-tree node]
[:and
[:string {:min 1 :max 255 :error/fn #(str (:value %) (tr "workspace.tokens.token-name-length-validation-error"))}]
(-> cto/schema:token-node-name
@ -164,20 +162,20 @@
current-name (:name node)
new-tokens (ctob/update-tokens-group active-tokens current-path current-name name)]
(and (some? new-tokens)
(some (fn [[token-name token]]
(not (ctob/token-name-path-exists? token-name tokens-lib set-id (ctob/get-id token))))
(some (fn [[token-name _]]
(not (ctob/token-name-path-exists? token-name tokens-tree)))
new-tokens))))]])
(def schema:token-description
[:string {:max 2048 :error/fn #(tr "errors.field-max-length" 2048)}])
(defn make-token-schema
[tokens-lib token-type set-id token-id]
[tokens-tree token-type]
[:and
(sm/merge
cto/schema:token-attrs
[:map
[:name (make-token-name-schema tokens-lib set-id token-id)]
[:name (make-token-name-schema tokens-tree)]
[:value (make-token-value-schema token-type)]
[:description {:optional true} schema:token-description]])
[:fn {:error/field :value
@ -187,9 +185,9 @@
(not (cto/token-value-self-reference? name value))))]])
(defn make-node-token-schema
[active-tokens tokens-lib node set-id]
[active-tokens tokens-tree node]
[:map
[:name (make-node-token-name-schema active-tokens tokens-lib node set-id)]])
[:name (make-node-token-name-schema active-tokens tokens-tree node)]])
(defn convert-dtcg-token
"Convert token attributes as they come from a decoded json, with DTCG types, to internal types.

124
common/src/app/common/types/tokens_lib.cljc
View File

@ -1484,63 +1484,49 @@ Will return a value that matches this schema:
(rename copy-name)
(reid (uuid/next))))))
(defn- token-name->path-selector
"Splits token-name into map with `:path` and `:selector` using `token-name->path`.
`:selector` is the last item of the names path
`:path` is everything leading up the the `:selector`."
[token-name]
(let [path-segments (get-token-path {:name token-name})
last-idx (dec (count path-segments))
[path [selector]] (split-at last-idx path-segments)]
{:path (seq path)
:selector selector}))
(defn token-name-path-exists?
"Check if a token name or fragment exists in any part of the library, to prevent creating
duplicated names that may clash when merging sets and resolving tokens.
"Traverses the path from `token-name` down a `tokens-tree` and checks if a token at that path exists.
Matches any combination of of names completely included inside group or token names.
For example:
- Matches the name \"foo.bar\" with an existing token named \"foo.bar.baz\" or \"foo\".
- Does not match the name \"foo.bar\" with an existing token named \"foo.baz\".
It's not allowed to create a token inside a token. E.g.:
Creating a token with
You can give a current set id, and it will check if there is a token with the exact same
name in this set (there may be tokens with same name in different sets for overriding
values, but not in the same set). You can also give a token id to ignore, to search for
a token that is a different one.
{:name \"foo.bar\"}
If the function finds a match, it returns the part of the name that is duplicated;
if not, it returns null."
[token-name tokens-lib current-set-id token-id-to-ignore]
(letfn [(exists-in-set?
[set]
(let [tokens-tree (-> set (get-tokens-) (tokens-tree))
token-name-path (get-token-path {:name token-name})]
(loop [path-segment token-name-path
subtree tokens-tree]
(if (empty? path-segment)
;; All path segments found -> return full name
token-name
(let [node (get subtree (first path-segment))]
(cond
;; Path segment doesn't exist
(nil? node) nil
;; A token exists at this path
(token? node)
(if (and (some? token-id-to-ignore)
(= (get-id node) token-id-to-ignore))
;; This is the token to ignore
nil
(if (and (not= (get-id set) current-set-id)
(= (get-name node) token-name))
;; A token with the same name in a different set is allowed
nil
;; If we are in the same set or the name of the token is a subpath of the
;; current name: this is a conflict
;; -> return the part of the name until this point
(str/join "." (take (- (count token-name-path) (count (rest path-segment)))
token-name-path))))
;; Continue traversing the tree
:else (recur (rest path-segment) node)))))))]
in the tokens tree:
(if (or (nil? tokens-lib) (empty? (get-sets tokens-lib))
(nil? token-name) (str/empty? token-name))
nil
(do
(assert (or (nil? current-set-id)
(some? (get-set tokens-lib current-set-id)))
(str "Set '" current-set-id "' does not exist in the library"))
(assert (or (nil? token-id-to-ignore) (uuid? token-id-to-ignore)))
(some exists-in-set? (get-sets tokens-lib))))))
{\"foo\" {:name \"other\"}}"
[token-name tokens-tree]
(let [{:keys [path selector]} (token-name->path-selector token-name)
path-target (reduce
(fn [acc cur]
(let [target (get acc cur)]
(cond
;; Path segment doesn't exist yet
(nil? target) (reduced false)
;; A token exists at this path
(:name target) (reduced true)
;; Continue traversing the true
:else target)))
tokens-tree
path)]
(cond
(boolean? path-target) path-target
(get path-target :name) true
:else (-> (get path-target selector)
(seq)
(boolean)))))
(defn update-tokens-group
"Updates the active tokens path when renaming a group node.
@ -1554,7 +1540,6 @@ Will return a value that matches this schema:
new-name: the new name for the group being renamed, e.g. \"baz\"
Returns a sequence of [name token] for each renamed token."
[active-tokens current-path current-name new-name]
(let [path-prefix (str/replace current-path current-name "")]
(mapv (fn [[token-path token-obj]]
@ -1927,11 +1912,7 @@ Will return a value that matches this schema:
library
(reduce (fn [library name]
(if-let [tokens (get sets name)]
(do (doseq [token (vals tokens)]
(when (token-name-path-exists? (get-name token) library nil (get-id token))
(throw (ex-info (get-name token)
{:error/code :error.import/duplicated-token-name}))))
(add-set library (make-token-set :name name :tokens tokens)))
(add-set library (make-token-set :name name :tokens tokens))
library))
library
ordered-set-names)
@ -2248,29 +2229,6 @@ Will return a value that matches this schema:
(map->tokens-lib)
(check)))))
(defn fix-conflicting-token-names
[tokens-lib]
(let [counter (atom 0)
match-suffixes (atom {})
generate-name
(fn [name match]
(let [matches (if (contains? @match-suffixes match)
@match-suffixes
(swap! match-suffixes assoc match (swap! counter inc)))
suffix (get matches match)]
(str (str/slice name 0 (count match))
"-" suffix
(str/slice name (count match)))))]
(update-all-tokens
tokens-lib
(fn [lib set token]
(let [name (get-name token)]
(if-let [match (token-name-path-exists? name lib (:id set) (get-id token))]
(rename token (generate-name name match))
token))))))
(defn fix-missing-sets-in-themes
[tokens-lib]
(let [existing-set-names (into #{} (map get-name) (get-sets tokens-lib))
@ -2321,7 +2279,7 @@ Will return a value that matches this schema:
#?(:clj
(defn- migrate-to-v1-3
"Migrate the TokensLib data structure internals to v1.3 version; it
expects input from v1.2 version"
expects input from v1.2 version"
[{:keys [sets themes] :as params}]
(let [migrate-token
(fn [token]
@ -2369,7 +2327,7 @@ Will return a value that matches this schema:
#?(:clj
(defn- migrate-to-v1-4
"Migrate the TokensLib data structure internals to v1.4 version; it
expects input from v1.3 version"
expects input from v1.3 version"
[params]
(let [migrate-set-node
(fn recurse [node]

235
common/test/common_tests/types/tokens_lib_test.cljc
View File

@ -2071,235 +2071,12 @@
(t/is (= (:value imported-ref) (:value original-ref))))))))
(t/deftest token-name-path-exists?-test
(let [tokens-lib (ctob/make-tokens-lib)
add-set (fn [lib set-label set-name token-names]
(ctob/add-set lib (ctob/make-token-set
:id (thi/new-id! set-label)
:name set-name
:tokens (into {}
(map (fn [token-name]
[token-name (ctob/make-token
{:name token-name
:type :border-radius
:value "1"})]))
token-names))))]
;; Empty cases
(t/testing "returns match for no library or empty library or empty name"
(t/is (not (ctob/token-name-path-exists? nil nil nil nil)))
(t/is (not (ctob/token-name-path-exists? nil tokens-lib nil nil)))
(t/is (not (ctob/token-name-path-exists? "" tokens-lib nil nil)))
(t/is (not (ctob/token-name-path-exists? "bad-name" tokens-lib nil nil)))
(t/is (not (ctob/token-name-path-exists? "bad-name"
(ctob/add-theme tokens-lib
(ctob/make-token-theme {:name "theme1"}))
nil
nil))))
(t/testing "throws error when giving a bad set id"
(t/is (thrown-with-msg? #?(:clj AssertionError :cljs js/Error)
#"Set '[0-9a-f-]+' does not exist in the library"
(ctob/token-name-path-exists? "some-name"
(-> tokens-lib
(add-set :empty-set "empty-set" []))
(thi/new-id! :non-existent-set) nil))))
(t/testing "does not throw error when giving a nil set id"
(t/is (not (ctob/token-name-path-exists? "some-name"
(-> tokens-lib
(add-set :empty-set "empty-set" []))
nil nil))))
(t/testing "returns not match for empty set"
(t/is (not (ctob/token-name-path-exists? "some-name"
(-> tokens-lib
(add-set :empty-set "empty-set" []))
(thi/id :empty-set) nil))))
;; Search in the current set
(t/testing "returns match when name matches exactly a token in the set without groups"
(t/is (= "token1"
(ctob/token-name-path-exists? "token1"
(-> tokens-lib
(add-set :set1 "set1" ["token1" "token2" "token3"]))
(thi/id :set1) nil))))
(t/testing "returns match when name matches exactly a token in the set with groups"
(t/is (= "group1.subgroup1.token2"
(ctob/token-name-path-exists? "group1.subgroup1.token2"
(-> tokens-lib
(add-set :set1 "set1" ["group1.subgroup1.token1"
"group1.subgroup1.token2"
"group2.subgroup2.token3"]))
(thi/id :set1) nil))))
(t/testing "returns match when name is a subpath of a token in the set"
(t/is (= "group1"
(ctob/token-name-path-exists? "group1"
(-> tokens-lib
(add-set :set1 "set1" ["group1.subgroup1.token1"
"group1.subgroup1.token2"
"group2.subgroup2.token3"]))
(thi/id :set1) nil)))
(t/is (= "group1.subgroup1"
(ctob/token-name-path-exists? "group1.subgroup1"
(-> tokens-lib
(add-set :set1 "set1" ["group1.subgroup1.token1"
"group1.subgroup1.token2"
"group2.subgroup2.token3"]))
(thi/id :set1) nil))))
(t/testing "returns match when one of the token names in the set is a subpath of the name"
(t/is (= "group2.subgroup2.token3"
(ctob/token-name-path-exists? "group2.subgroup2.token3.subtoken"
(-> tokens-lib
(add-set :set1 "set1" ["group1.subgroup1.token1"
"group1.subgroup1.token2"
"group2.subgroup2.token3"]))
(thi/id :set1) nil))))
(t/testing "returns not match when name matches part of the path but not the full token name"
(t/is (not (ctob/token-name-path-exists? "group1.subgroup1.token4"
(-> tokens-lib
(add-set :set1 "set1" ["group1.subgroup1.token1"
"group1.subgroup1.token2"
"group2.subgroup2.token3"]))
(thi/id :set1) nil))))
(t/testing "returns not match when name does not match any part of the token names in the set"
(t/is (not (ctob/token-name-path-exists? "token4"
(-> tokens-lib
(add-set :set1 "set1" ["group1.subgroup1.token1"
"group1.subgroup1.token2"
"group2.subgroup2.token3"]))
(thi/id :set1) nil))))
;; Search in other set
(t/testing "returns not match when name matches exactly a token in other set without groups"
(t/is (not (ctob/token-name-path-exists? "token1"
(-> tokens-lib
(add-set :set1 "set1" ["token1" "token2" "token3"])
(add-set :set2 "set2" []))
(thi/id :set2) nil))))
(t/testing "returns not match when name matches exactly a token in other set with groups"
(t/is (not (ctob/token-name-path-exists? "group1.subgroup1.token2"
(-> tokens-lib
(add-set :set1 "set1" ["group1.subgroup1.token1"
"group1.subgroup1.token2"
"group2.subgroup2.token3"])
(add-set :set2 "set2" []))
(thi/id :set2) nil))))
(t/testing "returns match when name is a subpath of a token in other set"
(t/is (= "group1"
(ctob/token-name-path-exists? "group1"
(-> tokens-lib
(add-set :set1 "set1" ["group1.subgroup1.token1"
"group1.subgroup1.token2"
"group2.subgroup2.token3"])
(add-set :set2 "set2" []))
(thi/id :set2) nil)))
(t/is (= "group1.subgroup1"
(ctob/token-name-path-exists? "group1.subgroup1"
(-> tokens-lib
(add-set :set1 "set1" ["group1.subgroup1.token1"
"group1.subgroup1.token2"
"group2.subgroup2.token3"])
(add-set :set2 "set2" []))
(thi/id :set2) nil))))
(t/testing "returns match when one of the token names in other set is a subpath of the name"
(t/is (= "group2.subgroup2.token3"
(ctob/token-name-path-exists? "group2.subgroup2.token3.subtoken"
(-> tokens-lib
(add-set :set1 "set1" ["group1.subgroup1.token1"
"group1.subgroup1.token2"
"group2.subgroup2.token3"])
(add-set :set2 "set2" []))
(thi/id :set2) nil))))
(t/testing "returns not match when name matches part of the path but not the full token name"
(t/is (not (ctob/token-name-path-exists? "group1.subgroup1.token4"
(-> tokens-lib
(add-set :set1 "set1" ["group1.subgroup1.token1"
"group1.subgroup1.token2"
"group2.subgroup2.token3"])
(add-set :set2 "set2" []))
(thi/id :set2) nil))))
(t/testing "returns not match when name does not match any part of the token names in the set"
(t/is (not (ctob/token-name-path-exists? "token4"
(-> tokens-lib
(add-set :set1 "set1" ["group1.subgroup1.token1"
"group1.subgroup1.token2"
"group2.subgroup2.token3"])
(add-set :set2 "set2" []))
(thi/id :set2) nil))))
;; Additional cases
(t/testing "returns match when matches an exact token with several sets"
(t/is (= "group3.subgroup3.token4"
(ctob/token-name-path-exists? "group3.subgroup3.token4"
(-> tokens-lib
(add-set :set1 "set1" ["group1.subgroup1.token1"
"group1.subgroup1.token2"
"group2.subgroup2.token3"])
(add-set :set2 "set2" ["group3.subgroup3.token4"]))
(thi/id :set2) nil))))
(t/testing "returns match when matches in one of the sets, even if the set is disabled"
(let [tokens-lib (-> tokens-lib
(add-set :set1 "set1" ["group1.subgroup1.token1"
"group1.subgroup1.token2"
"group2.subgroup2.token3"])
(add-set :set2 "set2" ["group3.subgroup3.token4"]))
hidden-theme (ctob/get-hidden-theme tokens-lib)
tokens-lib (ctob/toggle-set-in-theme tokens-lib (:id hidden-theme) "set2")]
(t/is (= "group3.subgroup3.token4"
(ctob/token-name-path-exists? "group3.subgroup3.token4"
tokens-lib
(thi/id :set2)
nil)))))
(t/testing "returns not match when does not match in any of the sets"
(t/is (not (ctob/token-name-path-exists? "group3.subgroup3.token5"
(-> tokens-lib
(add-set :set1 "set1" ["group1.subgroup1.token1"
"group1.subgroup1.token2"
"group2.subgroup2.token3"])
(add-set :set2 "set2" ["group3.subgroup3.token4"]))
(thi/id :set1)
nil))))
(t/testing "returns not match when the token exists but is the one we have told it to ignore"
(let [tokens-lib (-> tokens-lib
(add-set :set1 "set1" ["group1.subgroup1.token1"
"group1.subgroup1.token2"
"group2.subgroup2.token3"])
(add-set :set2 "set2" ["group3.subgroup3.token4"]))
token4 (ctob/get-token-by-name tokens-lib "set2" "group3.subgroup3.token4")]
(t/is (not (ctob/token-name-path-exists? "group3.subgroup3.token4"
tokens-lib
(thi/id :set2)
(:id token4))))))
(t/testing "returns match when we give an id to ignore but is not the token that matches"
(let [tokens-lib (-> tokens-lib
(add-set :set1 "set1" ["group1.subgroup1.token1"
"group1.subgroup1.token2"
"group2.subgroup2.token3"])
(add-set :set2 "set2" ["group3.subgroup3.token4"]))
token4 (ctob/get-token-by-name tokens-lib "set2" "group3.subgroup3.token4")]
(t/is (= "group1.subgroup1.token1"
(ctob/token-name-path-exists? "group1.subgroup1.token1"
tokens-lib
(thi/id :set1)
(:id token4))))))))
(t/is (true? (ctob/token-name-path-exists? "border-radius" {"border-radius" {"sm" {:name "sm"}}})))
(t/is (true? (ctob/token-name-path-exists? "border-radius" {"border-radius" {:name "sm"}})))
(t/is (true? (ctob/token-name-path-exists? "border-radius.sm" {"border-radius" {:name "sm"}})))
(t/is (true? (ctob/token-name-path-exists? "border-radius.sm.x" {"border-radius" {:name "sm"}})))
(t/is (false? (ctob/token-name-path-exists? "other" {"border-radius" {:name "sm"}})))
(t/is (false? (ctob/token-name-path-exists? "dark.border-radius.md" {"dark" {"border-radius" {"sm" {:name "sm"}}}}))))
#?(:clj
(t/deftest token-set-encode-decode-roundtrip-with-invalid-set-name

202
common/test/common_tests/types/tokens_migrations_test.cljc
View File

@ -13,205 +13,7 @@
[clojure.datafy :refer [datafy]]
[clojure.test :as t]))
(t/deftest test-v1-5-fix-token-names
(t/testing "empty tokens-lib should not need any action"
(let [tokens-lib (ctob/make-tokens-lib)
tokens-lib' (ctob/fix-conflicting-token-names tokens-lib)]
(t/is (empty? (d/map-diff (datafy tokens-lib) (datafy tokens-lib'))))))
(t/testing "tokens with valid names should not need any action"
(let [tokens-lib (-> (ctob/make-tokens-lib)
(ctob/add-set (ctob/make-token-set
:id (thi/new-id! :set1)
:name "set1"
:tokens {"name1" (ctob/make-token
{:id (thi/new-id! :token1)
:name "name1"
:type :border-radius
:value "1"})}))
(ctob/add-set (ctob/make-token-set
:id (thi/new-id! :set2)
:name "set2"
:tokens {"name1" (ctob/make-token ;; Same name in different
{:id (thi/new-id! :token2) ;; sets is ok
:name "name1"
:type :border-radius
:value "2"})})))
tokens-lib' (ctob/fix-conflicting-token-names tokens-lib)]
(t/is (empty? (d/map-diff (datafy tokens-lib) (datafy tokens-lib'))))))
(t/testing "tokens with conflicting names should be renamed, and the rest of the library should be unchanged"
(let [tokens-lib (-> (ctob/make-tokens-lib)
(ctob/add-set (ctob/make-token-set
:id (thi/new-id! :set1)
:name "set1"
:tokens {"name1" (ctob/make-token
{:id (thi/new-id! :token1)
:name "name1"
:type :border-radius
:value "1"})}))
(ctob/add-set (ctob/make-token-set
:id (thi/new-id! :set2)
:name "set2"
:tokens {"name1.name2" (ctob/make-token
{:id (thi/new-id! :token2)
:name "name1.name2"
:type :border-radius
:value "2"})})))
tokens-lib' (ctob/fix-conflicting-token-names tokens-lib)
token-sets (ctob/get-set-tree tokens-lib)
set1 (ctob/get-set tokens-lib (thi/id :set1))
set2 (ctob/get-set tokens-lib (thi/id :set2))
tokens1 (ctob/get-tokens tokens-lib (thi/id :set1))
tokens2 (ctob/get-tokens tokens-lib (thi/id :set2))
token1 (ctob/get-token tokens-lib (thi/id :set1) (thi/id :token1))
token2 (ctob/get-token tokens-lib (thi/id :set2) (thi/id :token2))
token-sets' (ctob/get-set-tree tokens-lib')
set1' (ctob/get-set tokens-lib' (thi/id :set1))
set2' (ctob/get-set tokens-lib' (thi/id :set2))
tokens1' (ctob/get-tokens tokens-lib' (thi/id :set1))
tokens2' (ctob/get-tokens tokens-lib' (thi/id :set2))
token1' (ctob/get-token tokens-lib' (thi/id :set1) (thi/id :token1))
token2' (ctob/get-token tokens-lib' (thi/id :set2) (thi/id :token2))]
(t/is (= (count token-sets') (count token-sets)))
(t/is (= (ctob/get-id set1') (ctob/get-id set1)))
(t/is (= (ctob/get-name set1') (ctob/get-name set1)))
(t/is (= (ctob/get-description set1') (ctob/get-description set1)))
(t/is (ct/is-after-or-equal? (ctob/get-modified-at set1') (ctob/get-modified-at set1))) ;; <-- MODIFIED
(t/is (= (ctob/get-id set2') (ctob/get-id set2)))
(t/is (= (ctob/get-name set2') (ctob/get-name set2)))
(t/is (= (ctob/get-description set2') (ctob/get-description set2)))
(t/is (= (ctob/get-modified-at set2') (ctob/get-modified-at set2)))
(t/is (= (count tokens1') (count tokens1)))
(t/is (= (count tokens2') (count tokens2)))
(t/is (= (ctob/get-id token1') (ctob/get-id token1)))
(t/is (= (ctob/get-name token1') "name1-1")) ;; <-- RENAMED
(t/is (= (ctob/get-description token1') (ctob/get-description token1)))
(t/is (ct/is-after-or-equal? (ctob/get-modified-at set1') (ctob/get-modified-at set1))) ;; <-- MODIFIED
(t/is (= (:type token1') (:type token1)))
(t/is (= (:value token1') (:value token1)))
(t/is (= (ctob/get-id token2') (ctob/get-id token2)))
(t/is (= (ctob/get-name token2') (ctob/get-name token2)))
(t/is (= (ctob/get-description token2') (ctob/get-description token2)))
(t/is (= (ctob/get-modified-at token2') (ctob/get-modified-at token2)))
(t/is (= (:type token2') (:type token2)))
(t/is (= (:value token2') (:value token2)))))
(t/testing "the renamed token is always the first one found with a conflicting name"
(let [tokens-lib (-> (ctob/make-tokens-lib)
(ctob/add-set (ctob/make-token-set
:id (thi/new-id! :set1)
:name "set1"
:tokens {"name1.name2" (ctob/make-token
{:id (thi/new-id! :token1)
:name "name1.name2"
:type :border-radius
:value "1"})}))
(ctob/add-set (ctob/make-token-set
:id (thi/new-id! :set2)
:name "set2"
:tokens {"name1" (ctob/make-token
{:id (thi/new-id! :token2)
:name "name1"
:type :border-radius
:value "2"})})))
tokens-lib' (ctob/fix-conflicting-token-names tokens-lib)
token1' (ctob/get-token tokens-lib' (thi/id :set1) (thi/id :token1))
token2' (ctob/get-token tokens-lib' (thi/id :set2) (thi/id :token2))]
(t/is (= "name1-1.name2" (ctob/get-name token1')))
(t/is (= "name1" (ctob/get-name token2')))))
(t/testing "several tokens with the same conflicting prefix should be assigned the same number as suffixes"
(let [tokens-lib (-> (ctob/make-tokens-lib)
(ctob/add-set (ctob/make-token-set
:id (thi/new-id! :set1)
:name "set1"
:tokens {"name1.name2" (ctob/make-token
{:id (thi/new-id! :token1)
:name "name1.name2"
:type :border-radius
:value "1"})
"name1.name3" (ctob/make-token
{:id (thi/new-id! :token2)
:name "name1.name3"
:type :border-radius
:value "2"})}))
(ctob/add-set (ctob/make-token-set
:id (thi/new-id! :set2)
:name "set2"
:tokens {"name1" (ctob/make-token
{:id (thi/new-id! :token3)
:name "name1"
:type :border-radius
:value "3"})})))
tokens-lib' (ctob/fix-conflicting-token-names tokens-lib)
token1' (ctob/get-token tokens-lib' (thi/id :set1) (thi/id :token1))
token2' (ctob/get-token tokens-lib' (thi/id :set1) (thi/id :token2))
token3' (ctob/get-token tokens-lib' (thi/id :set2) (thi/id :token3))]
(t/is (= "name1-1.name2" (ctob/get-name token1')))
(t/is (= "name1-1.name3" (ctob/get-name token2')))
(t/is (= "name1" (ctob/get-name token3')))))
(t/testing "tokens with diferent conflicting prefixes should be assigned consecutive numbers as suffixes"
(let [tokens-lib (-> (ctob/make-tokens-lib)
(ctob/add-set (ctob/make-token-set
:id (thi/new-id! :set1)
:name "set1"
:tokens {"name1" (ctob/make-token
{:id (thi/new-id! :token1)
:name "name1"
:type :border-radius
:value "1"})
"name2" (ctob/make-token
{:id (thi/new-id! :token2)
:name "name2"
:type :border-radius
:value "2"})}))
(ctob/add-set (ctob/make-token-set
:id (thi/new-id! :set2)
:name "set2"
:tokens {"name1.subname1" (ctob/make-token
{:id (thi/new-id! :token3)
:name "name1.subname1"
:type :border-radius
:value "3"})}))
(ctob/add-set (ctob/make-token-set
:id (thi/new-id! :set3)
:name "set3"
:tokens {"name2.subname2" (ctob/make-token
{:id (thi/new-id! :token4)
:name "name2.subname2"
:type :border-radius
:value "3"})})))
tokens-lib' (ctob/fix-conflicting-token-names tokens-lib)
token1' (ctob/get-token tokens-lib' (thi/id :set1) (thi/id :token1))
token2' (ctob/get-token tokens-lib' (thi/id :set1) (thi/id :token2))
token3' (ctob/get-token tokens-lib' (thi/id :set2) (thi/id :token3))
token4' (ctob/get-token tokens-lib' (thi/id :set3) (thi/id :token4))]
(t/is (= "name1-1" (ctob/get-name token1')))
(t/is (= "name2-2" (ctob/get-name token2')))
(t/is (= "name1.subname1" (ctob/get-name token3')))
(t/is (= "name2.subname2" (ctob/get-name token4'))))))
(t/deftest test-v1-6-fix-token-names
(t/deftest test-fix-missing-sets-in-themes
(t/testing "empty tokens-lib should not need any action"
(let [tokens-lib (ctob/make-tokens-lib)
@ -277,4 +79,4 @@
(t/is (= (ctob/get-description theme1') (ctob/get-description theme1)))
(t/is (= (ctob/get-id theme2') (ctob/get-id theme2)))
(t/is (= (ctob/get-name theme2') (ctob/get-name theme2)))
(t/is (= (ctob/get-description theme2') (ctob/get-description theme2))))))
(t/is (= (ctob/get-description theme2') (ctob/get-description theme2))))))

75
frontend/playwright/ui/specs/tokens/crud.spec.js
View File

@ -1721,72 +1721,29 @@ test.describe("Tokens - creation", () => {
// Submit button should remain disabled when value is empty
await expect(submitButton).toBeDisabled();
});
});
test("User cannot create token with a conflicting name in other set", async ({
page,
}) => {
const {
tokensUpdateCreateModal,
tokenThemesSetsSidebar,
tokensSidebar,
tokenContextMenuForToken,
} = await setupTokensFileRender(page);
test("User duplicate color token", async ({ page }) => {
const { tokensSidebar, tokenContextMenuForToken } =
await setupTokensFileRender(page);
await expect(tokensSidebar).toBeVisible();
await expect(tokensSidebar).toBeVisible();
await tokenThemesSetsSidebar
.getByRole("button", { name: "light", exact: true })
.click();
await unfoldTokenType(tokensSidebar, "color");
const tokensTabPanel = page.getByRole("tabpanel", { name: "tokens" });
await tokensTabPanel
.getByRole("button", { name: "Add Token: Color" })
.click();
const colorToken = tokensSidebar.getByRole("button", {
name: "colors.blue.100",
});
await expect(tokensUpdateCreateModal).toBeVisible();
await colorToken.click({ button: "right" });
await expect(tokenContextMenuForToken).toBeVisible();
const nameField = tokensUpdateCreateModal.getByLabel("Name");
const valueField = tokensUpdateCreateModal.getByLabel("Value");
const submitButton = tokensUpdateCreateModal.getByRole("button", {
name: "Save",
await tokenContextMenuForToken.getByText("Duplicate token").click();
await expect(tokenContextMenuForToken).not.toBeVisible();
await expect(
tokensSidebar.getByRole("button", { name: "colors.blue.100-copy" }),
).toBeVisible();
});
// Initially submit button should be disabled
await expect(submitButton).toBeDisabled();
await nameField.click();
// Fill in the name of an existing token in the current set
await nameField.fill("accent.default");
// An error message should appear and submit button should be disabled
await expect(
tokensUpdateCreateModal.getByText(
"A token already exists at the path: accent.default",
),
).toBeVisible();
await expect(submitButton).toBeDisabled();
// Fill in a name that clashes with tokens like colors.red.600 in set core
await nameField.fill("colors.red");
// An error message should appear and submit button should be disabled
await expect(
tokensUpdateCreateModal.getByText(
"A token already exists at the path: colors.red",
),
).toBeVisible();
await expect(submitButton).toBeDisabled();
// Fill in a name that matches exactly a token in another set
await nameField.fill("colors.red.600");
await valueField.fill("#6000000");
// Submit button should be enabled now
await expect(submitButton).toBeEnabled();
});
test("User creates grouped color token", async ({ page }) => {

63
frontend/playwright/ui/specs/tokens/tree.spec.js
View File

@ -275,4 +275,65 @@ test.describe("Tokens - node tree", () => {
await expect(tokenTypeButton).toHaveAttribute("aria-expanded", "false");
});
});
});
test("User can see an error on token pill and token modal form when token has an error", async ({
page,
}) => {
const {
tokensSidebar,
tokensUpdateCreateModal,
tokenContextMenuForToken,
tokenThemesSetsSidebar,
} = await setupTokensFileRender(page);
await createSet(tokenThemesSetsSidebar, "set/first");
await tokenThemesSetsSidebar.getByRole("button", { name: "first" }).click();
await tokenThemesSetsSidebar
.getByRole("button", { name: "first" })
.getByRole("checkbox")
.click();
await createSet(tokenThemesSetsSidebar, "set/second");
await tokenThemesSetsSidebar.getByRole("button", { name: "second" }).click();
await tokenThemesSetsSidebar
.getByRole("button", { name: "second" })
.getByRole("checkbox")
.click();
await createToken(page, "Border radius", "a.b", "Value", "textbox", "23");
await tokenThemesSetsSidebar.getByRole("button", { name: "first" }).click();
await createToken(page, "Border radius", "a", "Value", "textbox", "25");
await tokenThemesSetsSidebar.getByRole("button", { name: "second" }).click();
const brokenTokenPill = tokensSidebar.getByRole("button", {
name: "Group name of a.b conflicts",
});
await expect(brokenTokenPill).toBeVisible();
await brokenTokenPill.click({ button: "right" });
const editTokenButton = page
.getByRole("listitem")
.filter({ hasText: "Edit token" });
await expect(editTokenButton).toBeVisible();
await editTokenButton.click();
const nameField = tokensUpdateCreateModal.getByLabel("Name");
await expect(nameField).toBeVisible();
await expect(nameField).toHaveValue("a.b");
const errorMessage = tokensUpdateCreateModal.getByText(
"Group name of a.b conflicts",
);
await expect(errorMessage).toBeVisible();
await nameField.fill("new-name");
await expect(errorMessage).not.toBeVisible();
const submitButton = tokensUpdateCreateModal.getByRole("button", {
name: "Save",
});
await expect(submitButton).toBeEnabled();
});

5
frontend/src/app/main/data/workspace/tokens/errors.cljs
View File

@ -27,11 +27,6 @@
:error/fn #(tr "errors.tokens.invalid-json-token-name")
:error/detail #(tr "errors.tokens.invalid-json-token-name-detail" %)}
:error.import/duplicated-token-name
{:error/code :error.import/duplicated-token-name
:error/fn #(tr "workspace.tokens.duplicated-json-token-name")
:error/detail #(tr "workspace.tokens.duplicated-json-token-name-detail" %)}
:error.import/style-dictionary-reference-errors
{:error/code :error.import/style-dictionary-reference-errors
:error/fn #(str (tr "errors.tokens.import-error") "\n\n" (first %))

9
frontend/src/app/main/ui/workspace/tokens/management/forms/color.cljs
View File

@ -8,13 +8,12 @@
(:require
[app.common.files.tokens :as cfo]
[app.common.schema :as sm]
[app.common.types.tokens-lib :as ctob]
[app.main.ui.workspace.tokens.management.forms.controls :as token.controls]
[app.main.ui.workspace.tokens.management.forms.generic-form :as generic]
[rumext.v2 :as mf]))
(mf/defc form*
[{:keys [token token-type selected-token-set-id] :as props}]
[{:keys [token token-type] :as props}]
(let [initial
(mf/with-memo [token-type token]
{:type token-type
@ -23,11 +22,7 @@
:description (:description token "")
:color-result ""})
props (mf/spread-props props {:make-schema #(-> (cfo/make-token-schema %1
token-type
selected-token-set-id
(when (ctob/token? token)
(ctob/get-id token)))
props (mf/spread-props props {:make-schema #(-> (cfo/make-token-schema %1 token-type)
(sm/dissoc-key :id)
(sm/assoc-key :color-result :string))
:initial initial

9
frontend/src/app/main/ui/workspace/tokens/management/forms/font_family.cljs
View File

@ -9,7 +9,6 @@
[app.common.files.tokens :as cfo]
[app.common.schema :as sm]
[app.common.types.token :as cto]
[app.common.types.tokens-lib :as ctob]
[app.main.data.workspace.tokens.errors :as wte]
[app.main.ui.workspace.tokens.management.forms.controls :as token.controls]
[app.main.ui.workspace.tokens.management.forms.generic-form :as generic]
@ -30,7 +29,7 @@
(default-validate-token)))
(mf/defc form*
[{:keys [token token-type selected-token-set-id] :rest props}]
[{:keys [token token-type] :rest props}]
(let [token
(mf/with-memo [token]
(if token
@ -38,11 +37,7 @@
{:type token-type}))
props (mf/spread-props props {:token token
:token-type token-type
:make-schema #(-> (cfo/make-token-schema %1
token-type
selected-token-set-id
(when (ctob/token? token)
(ctob/get-id token)))
:make-schema #(-> (cfo/make-token-schema %1 token-type)
(sm/dissoc-key :id)
;; The value as edited in the form is a simple stirng.
;; It's converted to vector in the validator.

6
frontend/src/app/main/ui/workspace/tokens/management/forms/form_container.cljs
View File

@ -27,13 +27,13 @@
selected-token-set-id
(mf/deref refs/selected-token-set-id)
tokens-in-selected-set
(mf/deref refs/workspace-all-tokens-in-selected-set)
token-path
(mf/with-memo [token]
(ctob/get-token-path token))
tokens-in-selected-set
(mf/deref refs/workspace-all-tokens-in-selected-set)
tokens-tree-in-selected-set
(mf/with-memo [token-path tokens-in-selected-set]
(-> (ctob/tokens-tree tokens-in-selected-set)

13
frontend/src/app/main/ui/workspace/tokens/management/forms/generic_form.cljs
View File

@ -69,6 +69,7 @@
action
is-create
selected-token-set-id
tokens-tree-in-selected-set
token-type
make-schema
input-component
@ -78,11 +79,7 @@
value-subfield
input-value-placeholder] :as props}]
(let [make-schema (or make-schema #(-> (cfo/make-token-schema %
token-type
selected-token-set-id
(when (ctob/token? token)
(ctob/get-id token)))
(let [make-schema (or make-schema #(-> (cfo/make-token-schema % token-type)
(sm/dissoc-key :id)))
input-component (or input-component token.controls/input*)
validate-token (or validator default-validate-token)
@ -99,8 +96,6 @@
token-title (str/lower (:title token-properties))
tokens-lib (mf/deref refs/tokens-lib)
;; All tokens in the lib, as a map name -> token, flattened
;; including tokens in inactive sets.
tokens-tree (mf/deref refs/workspace-all-tokens-map)
@ -138,8 +133,8 @@
resolved-active-tokens))))
schema
(mf/with-memo [tokens-lib active-tab]
(make-schema tokens-lib active-tab))
(mf/with-memo [tokens-tree-in-selected-set active-tab]
(make-schema tokens-tree-in-selected-set active-tab))
initial
(mf/with-memo [token initial]

16
frontend/src/app/main/ui/workspace/tokens/management/forms/rename_node_modal.cljs
View File

@ -3,8 +3,8 @@
(:require
[app.common.data :as d]
[app.common.files.tokens :as cfo]
[app.common.types.tokens-lib :as ctob]
[app.main.data.modal :as modal]
[app.main.refs :as refs]
[app.main.store :as st]
[app.main.ui.ds.buttons.button :refer [button*]]
[app.main.ui.ds.buttons.icon-button :refer [icon-button*]]
@ -18,8 +18,8 @@
[rumext.v2 :as mf]))
(mf/defc rename-node-form*
[{:keys [new-node-name node active-tokens tokens-lib selected-token-set-id variant on-close on-submit]}]
(let [make-schema #(cfo/make-node-token-schema active-tokens tokens-lib node selected-token-set-id)
[{:keys [new-node-name node active-tokens tokens-tree variant on-close on-submit]}]
(let [make-schema #(cfo/make-node-token-schema active-tokens tokens-tree node)
schema
(mf/with-memo [active-tokens]
@ -82,9 +82,10 @@
(let [variant (d/nilv variant "rename") ;; "rename" or "duplicate"
selected-token-set-id (mf/deref refs/selected-token-set-id)
tokens-lib (mf/deref refs/tokens-lib)
tokens-tree-in-selected-set
(mf/with-memo [tokens-in-active-set node]
(-> (ctob/tokens-tree tokens-in-active-set)
(d/dissoc-in (:name node))))
close-modal
(mf/use-fn
@ -117,7 +118,6 @@
:node node
:variant variant
:active-tokens tokens-in-active-set
:tokens-lib tokens-lib
:selected-token-set-id selected-token-set-id
:tokens-tree tokens-tree-in-selected-set
:on-close close-modal
:on-submit rename}]]]))

13
frontend/src/app/main/ui/workspace/tokens/management/forms/shadow.cljs
View File

@ -260,7 +260,7 @@
;; TODO: use cfo/make-schema:token-value and extend it with shadow and reference fields
(defn- make-schema
[set-id token-id tokens-lib active-tab]
[tokens-tree active-tab]
(sm/schema
[:and
[:map
@ -271,7 +271,7 @@
(sm/update-properties cto/schema:token-name assoc
:error/fn #(str (:value %) (tr "workspace.tokens.token-name-validation-error")))
[:fn {:error/fn #(tr "workspace.tokens.token-name-duplication-validation-error" (:value %))}
#(not (ctob/token-name-path-exists? % tokens-lib set-id token-id))]]]
#(not (ctob/token-name-path-exists? % tokens-tree))]]]
[:value
[:map
@ -348,7 +348,8 @@
:shadow [default-token-shadow]}))
(mf/defc form*
[{:keys [token token-type selected-token-set-id] :as props}]
[{:keys [token
token-type] :as props}]
(let [token
(mf/with-memo [token]
(or token
@ -359,12 +360,6 @@
{:type token-type
:value {:reference nil
:shadow [default-token-shadow]}})))
make-schema
(mf/with-memo [selected-token-set-id token]
(partial make-schema selected-token-set-id (when (ctob/token? token)
(ctob/get-id token))))
initial
(mf/with-memo [token]
(let [raw-value (:value token)

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

@ -209,7 +209,7 @@
;; TODO: use cfo/make-schema:token-value and extend it with typography and reference fields
(defn- make-schema
[set-id token-id tokens-lib active-tab]
[tokens-tree active-tab]
(sm/schema
[:and
[:map
@ -220,7 +220,7 @@
(sm/update-properties cto/schema:token-name assoc
:error/fn #(str (:value %) (tr "workspace.tokens.token-name-validation-error")))
[:fn {:error/fn #(tr "workspace.tokens.token-name-duplication-validation-error" (:value %))}
#(not (ctob/token-name-path-exists? % tokens-lib set-id token-id))]]]
#(not (ctob/token-name-path-exists? % tokens-tree))]]]
[:value
[:map
@ -269,7 +269,7 @@
result))]]))
(mf/defc form*
[{:keys [token selected-token-set-id] :as props}]
[{:keys [token] :as props}]
(let [initial
(mf/with-memo [token]
(let [value (:value token)
@ -296,12 +296,6 @@
{:name (:name token "")
:value processed-value
:description (:description token "")}))
make-schema
(mf/with-memo [selected-token-set-id token]
(partial make-schema selected-token-set-id (when (ctob/token? token)
(ctob/get-id token))))
props (mf/spread-props props {:initial initial
:make-schema make-schema
:token token

5
frontend/src/app/main/ui/workspace/tokens/management/group.cljs
View File

@ -132,7 +132,7 @@
on-popover-open-click
(mf/use-fn
(mf/deps type title modal selected-token-set-id)
(mf/deps type title modal)
(fn [event]
(dom/stop-propagation event)
(st/emit!
@ -144,8 +144,7 @@
:fields (:fields modal)
:title title
:action "create"
:token-type type
:selected-token-set-id selected-token-set-id})))))
:token-type type})))))
on-token-pill-click
(mf/use-fn

28
frontend/src/app/plugins/tokens.cljs
View File

@ -122,9 +122,7 @@
(ctob/get-name token)))
:schema (cfo/make-token-name-schema
(some-> (u/locate-tokens-lib file-id)
(ctob/get-tokens set-id))
set-id
id)
(ctob/get-tokens set-id)))
:set
(fn [_ value]
(st/emit! (-> (dwtl/update-token set-id id {:name value})
@ -313,17 +311,19 @@
:addToken
{:enumerable false
:schema (fn [args]
[:tuple (-> (cfo/make-token-schema
(u/locate-tokens-lib file-id)
(cto/dtcg-token-type->token-type (-> args (first) (get "type")))
id
(-> args (first) (get "id")))
;; 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))])
(let [tokens-tree (-> (u/locate-tokens-lib file-id)
(ctob/get-tokens id)
;; Convert to the adecuate format for schema
(ctob/tokens-tree))]
[:tuple (-> (cfo/make-token-schema
tokens-tree
(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)

4
frontend/translations/en.po
View File

@ -1917,6 +1917,10 @@ msgid "errors.tokens.value-with-units"
msgstr "Invalid value: Units are not allowed."
#: src/app/main/data/media.cljs:74
msgid "errors.token-theme-not-existing-sets"
msgstr "The theme refers to some not existing sets: %s"
#: src/app/main/data/media.cljs:73
msgid "errors.unexpected-error"
msgstr "An unexpected error occurred."

4
frontend/translations/es.po
View File

@ -1871,6 +1871,10 @@ msgid "errors.tokens.value-with-units"
msgstr "Valor no válido: No se permiten unidades."
#: src/app/main/data/media.cljs:74
msgid "errors.token-theme-not-existing-sets"
msgstr "El tema referencia sets que no existen: %s"
#: src/app/main/data/media.cljs:73
msgid "errors.unexpected-error"
msgstr "Ha ocurrido un error inesperado."