Merge pull request #211 from tokens-studio/fix-sub-name-space

Fix token naming clashes

frontend/src/app/main/ui/workspace/tokens/CHANGELOG.md

@@ -18,6 +18,27 @@ If possible add video here from PR as well
 
 ## Changes
 
+### 2024-07-01 - Disallow creating tokens at existing paths
+
+Disallow creating tokens at an existing path.
+
+[Video](https://github.com/tokens-studio/tokens-studio-for-penpot/assets/1898374/557990fe-efe7-445b-8a1d-824396049db7
+)
+
+Example:
+We've got a token with `borderRadius.sm`, so we can't allow to create a token at `borderRadius` or `borderRadius.sm`.
+But we can allow creating a token at `borderRadius.md`.
+
+
+### 2024-06-26 - Disallow special characters in token name
+
+- Only allows digits, letters and `-` as a part of a token name
+
+[Video](https://github.com/tokens-studio/tokens-studio-for-penpot/assets/1898374/7c59c0cc-d6e1-4b0d-9646-9a27eafcccc4
+)
+
+https://github.com/tokens-studio/tokens-studio-for-penpot/pull/200
+
 ### 2024-06-26 - Make Tokens JSON Export DTCG compatible
 
 ![Screenshot of sample JSON Export in DTCG format](https://private-user-images.githubusercontent.com/9948167/343043570-b4bb39f7-ec53-409a-a053-b284d60848d9.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTk0MDMyMzcsIm5iZiI6MTcxOTQwMjkzNywicGF0aCI6Ii85OTQ4MTY3LzM0MzA0MzU3MC1iNGJiMzlmNy1lYzUzLTQwOWEtYTA1My1iMjg0ZDYwODQ4ZDkucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI0MDYyNiUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNDA2MjZUMTE1NTM3WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9MWEzZTU5OWQ0M2JkZWE5MTA5MDc4MTY1OTkyZWE5MmE5YzBlYmQ2NTcwMmEwZTdmMjViNGU5YTFjNWIxYjU5ZCZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QmYWN0b3JfaWQ9MCZrZXlfaWQ9MCZyZXBvX2lkPTAifQ.qWJxRa_Y7LZ6EDJg5yPdOUIQkURFmZwMNec_BbdH9Co)

frontend/src/app/main/ui/workspace/tokens/form.cljs

@@ -8,11 +8,13 @@
   (:require-macros [app.main.style :as stl])
   (:require
    ["lodash.debounce" :as debounce]
+   [app.common.data :as d]
    [app.main.data.modal :as modal]
    [app.main.data.tokens :as dt]
    [app.main.store :as st]
    [app.main.ui.workspace.tokens.common :as tokens.common]
    [app.main.ui.workspace.tokens.style-dictionary :as sd]
+   [app.main.ui.workspace.tokens.token :as wtt]
    [app.util.dom :as dom]
    [cuerdas.core :as str]
    [malli.core :as m]
@@ -22,20 +24,35 @@
 
 ;; Schemas ---------------------------------------------------------------------
 
+(def valid-token-name-regexp
+  "Only allow letters and digits for token names.
+  Also allow one `.` for a namespace separator.
+
+  Caution: This will allow a trailing dot like `token-name.`,
+  But we will trim that in the `finalize-name`,
+  to not throw too many errors while the user is editing."
+  #"([a-zA-Z0-9-]+\.?)*")
+
+(def valid-token-name-schema
+  (m/-simple-schema
+   {:type :token/invalid-token-name
+    :pred #(re-matches valid-token-name-regexp %)
+    :type-properties {:error/fn #(str (:value %) " is not a valid token name.
+Token names should only contain letters and digits separated by . characters.")}}))
+
 (defn token-name-schema
-  "Generate a dynamic schema validation to check if a token name already exists.
-  `existing-token-names` should be a set of strings."
-  [existing-token-names]
-  (let [non-existing-token-schema
+  "Generate a dynamic schema validation to check if a token path derived from the name already exists at `tokens-tree`."
+  [{:keys [token tokens-tree]}]
+  (let [path-exists-schema
         (m/-simple-schema
          {:type :token/name-exists
-          :pred #(not (get existing-token-names %))
-          :type-properties {:error/fn #(str (:value %) " is an already existing token name")
-                            :existing-token-names existing-token-names}})]
+          :pred #(not (wtt/token-name-path-exists? % tokens-tree))
+          :type-properties {:error/fn #(str "A token already exists at the path: " (:value %))}})]
     (m/schema
      [:and
       [:string {:min 1 :max 255}]
-      non-existing-token-schema])))
+      valid-token-name-schema
+      path-exists-schema])))
 
 (def token-description-schema
   (m/schema
@@ -44,7 +61,9 @@
 ;; Helpers ---------------------------------------------------------------------
 
 (defn finalize-name [name]
-  (str/trim name))
+  (-> (str/trim name)
+      ;; Remove trailing dots
+      (str/replace #"\.+$" "")))
 
 (defn valid-name? [name]
   (seq (finalize-name (str name))))
@@ -81,7 +100,7 @@
                   new-tokens (update tokens token-id merge {:id token-id
                                                             :value input
                                                             :name token-name})]
-              (-> (sd/resolve-tokens+ new-tokens)
+              (-> (sd/resolve-tokens+ new-tokens {:debug? true})
                   (p/then
                    (fn [resolved-tokens]
                      (let [{:keys [errors resolved-value] :as resolved-token} (get resolved-tokens token-id)]
@@ -122,21 +141,24 @@
   {::mf/wrap-props false}
   [{:keys [token token-type] :as _args}]
   (let [tokens (sd/use-resolved-workspace-tokens)
-        existing-token-names (mf/use-memo
-                              (mf/deps tokens)
-                              (fn []
-                                (-> (into #{} (map (fn [[_ {:keys [name]}]] name) tokens))
-                                    ;; Remove the currently editing token name,
-                                    ;; as we don't want it to show when checking for duplicate names.
-                                    (disj (:name token)))))
+        token-path (mf/use-memo
+                    (mf/deps (:name token))
+                    #(wtt/token-name->path (:name token)))
+        tokens-tree (mf/use-memo
+                     (mf/deps token-path tokens)
+                     (fn []
+                       (-> (wtt/token-names-tree tokens)
+                           ;; Allow setting editing token to it's own path
+                           (d/dissoc-in token-path))))
 
         ;; Name
         name-ref (mf/use-var (:name token))
         name-errors (mf/use-state nil)
         validate-name (mf/use-callback
-                       (mf/deps existing-token-names)
+                       (mf/deps tokens-tree)
                        (fn [value]
-                         (let [schema (token-name-schema existing-token-names)]
+                         (let [schema (token-name-schema {:token token
+                                                          :tokens-tree tokens-tree})]
                            (m/explain schema (finalize-name value)))))
         on-update-name-debounced (mf/use-callback
                                   (debounce (fn [e]
@@ -235,9 +257,12 @@
                                                       :auto-focus true
                                                       :on-blur on-update-name
                                                       :on-change on-update-name}}]
-       (when @name-errors
-         [:p {:class (stl/css :error)}
-          (me/humanize @name-errors)])]
+       (for [error (->> (:errors @name-errors)
+                        (map #(-> (assoc @name-errors :errors [%])
+                                  (me/humanize))))]
+         [:p {:key error
+              :class (stl/css :error)}
+          error])]
       [:& tokens.common/labeled-input {:label "Value"
                                        :input-props {:default-value @value-ref
                                                      :on-blur on-update-value

frontend/src/app/main/ui/workspace/tokens/style_dictionary.cljs

@@ -4,7 +4,7 @@
    ["style-dictionary$default" :as sd]
    [app.common.data :as d]
    [app.main.refs :as refs]
-   [app.util.dom :as dom]
+   [app.main.ui.workspace.tokens.token :as wtt]
    [cuerdas.core :as str]
    [promesa.core :as p]
    [rumext.v2 :as mf]
@@ -82,14 +82,9 @@
   (and (set? errors)
        (get errors :style-dictionary/missing-reference)))
 
-(defn tokens-name-map [tokens]
-  (->> tokens
-       (map (fn [[_ x]] [(:name x) x]))
-       (into {})))
-
 (defn resolve-tokens+
   [tokens & {:keys [debug?] :as config}]
-  (p/let [sd-tokens (-> (tokens-name-map tokens)
+  (p/let [sd-tokens (-> (wtt/token-names-tree tokens)
                         (resolve-sd-tokens+ config))]
     (let [resolved-tokens (reduce
                            (fn [acc ^js cur]

frontend/src/app/main/ui/workspace/tokens/token.cljs

@@ -0,0 +1,63 @@
+(ns app.main.ui.workspace.tokens.token
+  (:require
+   [cuerdas.core :as str]))
+
+(defn token-name->path
+  "Splits token-name into a path vector split by `.` characters.
+
+  Will concatenate multiple `.` characters into one."
+  [token-name]
+  (str/split token-name #"\.+"))
+
+(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 (token-name->path token-name)
+        last-idx (dec (count path-segments))
+        [path [selector]] (split-at last-idx path-segments)]
+    {:path (seq path)
+     :selector selector}))
+
+(defn token-names-tree
+  "Convert tokens into a nested tree with their `:name` as the path."
+  [tokens]
+  (reduce
+   (fn [acc [_ {:keys [name] :as token}]]
+     (when (string? name)
+       (let [path (token-name->path name)]
+         (assoc-in acc path token))))
+   {} tokens))
+
+(defn token-name-path-exists?
+  "Traverses the path from `token-name` down a `token-tree` and checks if a token at that path exists.
+
+  It's not allowed to create a token inside a token. E.g.:
+  Creating a token with
+
+    {:name \"foo.bar\"}
+
+  in the tokens tree:
+
+    {\"foo\" {:name \"other\"}}"
+  [token-name token-names-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)))
+                     token-names-tree path)]
+    (cond
+      (boolean? path-target) path-target
+      (get path-target :name) true
+      :else (-> (get path-target selector)
+                (seq)
+                (boolean)))))

frontend/test/token_tests/token_form_test.cljs

@@ -0,0 +1,26 @@
+;; This Source Code Form is subject to the terms of the Mozilla Public
+;; License, v. 2.0. If a copy of the MPL was not distributed with this
+;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
+;;
+;; Copyright (c) KALEIDOS INC
+
+(ns token-tests.token-form-test
+  (:require
+   [app.main.ui.workspace.tokens.form :as wtf]
+   [cljs.test :as t :include-macros true]
+   [malli.core :as m]))
+
+(t/deftest test-valid-token-name-schema
+  ;; Allow regular namespace token names
+  (t/is (some? (m/validate wtf/valid-token-name-schema "Foo")))
+  (t/is (some? (m/validate wtf/valid-token-name-schema "foo")))
+  (t/is (some? (m/validate wtf/valid-token-name-schema "FOO")))
+  (t/is (some? (m/validate wtf/valid-token-name-schema "Foo.Bar.Baz")))
+  ;; Allow trailing tokens
+  (t/is (nil? (m/validate wtf/valid-token-name-schema "Foo.Bar.Baz....")))
+  ;; Disallow multiple separator dots
+  (t/is (nil? (m/validate wtf/valid-token-name-schema "Foo..Bar.Baz")))
+  ;; Disallow any special characters
+  (t/is (nil? (m/validate wtf/valid-token-name-schema "Hey Foo.Bar")))
+  (t/is (nil? (m/validate wtf/valid-token-name-schema "Hey😈Foo.Bar")))
+  (t/is (nil? (m/validate wtf/valid-token-name-schema "Hey%Foo.Bar"))))

frontend/test/token_tests/token_test.cljs

@@ -0,0 +1,36 @@
+;; This Source Code Form is subject to the terms of the Mozilla Public
+;; License, v. 2.0. If a copy of the MPL was not distributed with this
+;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
+;;
+;; Copyright (c) KALEIDOS INC
+
+(ns token-tests.token-test
+  (:require
+   [app.main.ui.workspace.tokens.token :as wtt]
+   [cljs.test :as t :include-macros true]))
+
+(t/deftest name->path-test
+  (t/is (= ["foo" "bar" "baz"] (wtt/token-name->path "foo.bar.baz")))
+  (t/is (= ["foo" "bar" "baz"] (wtt/token-name->path "foo..bar.baz")))
+  (t/is (= ["foo" "bar" "baz"] (wtt/token-name->path "foo..bar.baz...."))))
+
+(t/deftest tokens-name-tree-test
+  (t/is (= {"foo"
+            {"bar"
+             {"baz" {:name "foo.bar.baz", :value "a"},
+              "bam" {:name "foo.bar.bam", :value "b"}}},
+            "baz" {"bar" {"foo" {:name "baz.bar.foo", :value "{foo.bar.baz}"}}}}
+           (wtt/token-names-tree {:a {:name "foo.bar.baz"
+                                      :value "a"}
+                                  :b {:name "foo.bar.bam"
+                                      :value "b"}
+                                  :c {:name "baz.bar.foo"
+                                      :value "{foo.bar.baz}"}}))))
+
+(t/deftest token-name-path-exists?-test
+  (t/is (true? (wtt/token-name-path-exists? "border-radius" {"border-radius" {"sm" {:name "sm"}}})))
+  (t/is (true? (wtt/token-name-path-exists? "border-radius" {"border-radius" {:name "sm"}})))
+  (t/is (true? (wtt/token-name-path-exists? "border-radius.sm" {"border-radius" {:name "sm"}})))
+  (t/is (true? (wtt/token-name-path-exists? "border-radius.sm.x" {"border-radius" {:name "sm"}})))
+  (t/is (false? (wtt/token-name-path-exists? "other" {"border-radius" {:name "sm"}})))
+  (t/is (false? (wtt/token-name-path-exists? "dark.border-radius.md" {"dark" {"border-radius" {"sm" {:name "sm"}}}}))))