mirror of
https://github.com/penpot/penpot.git
synced 2026-07-24 15:08:24 +00:00
🐛 Fix token creation fail when set inactive (#10297)
* 🐛 Fix token creatin fail when set inactive * 🎉 Add a enable flag to addSet to enable the token set
This commit is contained in:
parent
4a41b2e5e0
commit
ecabe7ec32
@ -338,7 +338,14 @@
|
||||
:fn (fn [attrs]
|
||||
(let [tokens-lib (u/locate-tokens-lib file-id)
|
||||
token (ctob/make-token attrs)
|
||||
tokens-tree (-> (ctob/get-tokens-in-active-sets tokens-lib)
|
||||
;; Resolve against all tokens in the library (including those
|
||||
;; in inactive sets) so that references to structurally
|
||||
;; existing tokens resolve even if their set is not active.
|
||||
;; The target set's tokens take precedence over equally named
|
||||
;; tokens in other sets, and the new token takes precedence
|
||||
;; over all.
|
||||
tokens-tree (-> (merge (ctob/get-all-tokens-map tokens-lib)
|
||||
(ctob/get-tokens tokens-lib id))
|
||||
(assoc (:name token) token))
|
||||
resolved-tokens (ts/resolve-tokens tokens-tree)
|
||||
|
||||
@ -528,12 +535,27 @@
|
||||
:schema [:tuple (-> (sm/schema (cfo/make-token-set-schema
|
||||
(u/locate-tokens-lib file-id)
|
||||
nil))
|
||||
(sm/dissoc-key :id))] ;; We don't allow plugins to set the id
|
||||
(sm/dissoc-key :id) ;; We don't allow plugins to set the id
|
||||
;; Allow an optional `active` flag so a plugin can create
|
||||
;; an already-active set in a single call. Newly created
|
||||
;; sets are inactive by default (only active sets affect
|
||||
;; shapes and reference resolution). `active` is not part
|
||||
;; of the token-set data model, so the :fn strips it and
|
||||
;; applies it through the set-activation logic.
|
||||
(sm/merge [:map [:active {:optional true} ::sm/boolean]]))]
|
||||
|
||||
:fn (fn [attrs]
|
||||
(let [attrs (update attrs :name ctob/normalize-set-name)
|
||||
set (ctob/make-token-set attrs)]
|
||||
(let [active? (boolean (:active attrs))
|
||||
attrs (-> attrs
|
||||
(dissoc :active)
|
||||
(update :name ctob/normalize-set-name))
|
||||
set (ctob/make-token-set attrs)]
|
||||
(st/emit! (dwtl/create-token-set set))
|
||||
;; Newly created sets are inactive by default; activate it when
|
||||
;; requested. Enabling only adds the set name to the hidden theme,
|
||||
;; so it does not depend on the create event having propagated yet.
|
||||
(when active?
|
||||
(st/emit! (dwtl/set-enabled-token-set (ctob/get-name set) true)))
|
||||
;; Pass the set name as `initial-name` so the proxy can resolve
|
||||
;; it immediately, before the async `st/emit!` above propagates
|
||||
;; the new set into `@st/state`.
|
||||
|
||||
@ -11,6 +11,7 @@
|
||||
[app.common.test-helpers.ids-map :as cthi]
|
||||
[app.common.test-helpers.tokens :as ctht]
|
||||
[app.common.types.tokens-lib :as ctob]
|
||||
[app.main.data.tokenscript :as ts]
|
||||
[app.main.store :as st]
|
||||
[app.plugins.api :as api]
|
||||
[app.plugins.tokens :as ptok]
|
||||
@ -148,3 +149,80 @@
|
||||
(t/is (false? (boolean (ptok/token-attr? :not-a-real-attr))))
|
||||
(t/is (false? (boolean (ptok/token-attr? "not-a-real-attr"))))
|
||||
(t/is (false? (boolean (ptok/token-attr? nil)))))
|
||||
|
||||
;; Regression coverage for issue #10070.
|
||||
;;
|
||||
;; The Plugin API's `addToken` rejected reference tokens whose target
|
||||
;; lives in an *inactive* token set, even though the referenced token
|
||||
;; exists structurally. The proxy `:fn` resolved the new token against
|
||||
;; active sets only (`get-tokens-in-active-sets`), so a reference into an
|
||||
;; inactive set never resolved and fell into the generic `not-valid`
|
||||
;; error path.
|
||||
;;
|
||||
;; The fix resolves against *all* tokens in the library (inactive sets
|
||||
;; included), mirroring the workspace token-creation form. These tests
|
||||
;; reproduce the exact `tokens-tree` construction from both the buggy and
|
||||
;; the fixed `addToken` `:fn` and assert resolution behaviour directly —
|
||||
;; the proxy `:fn` itself drives the global store and `st/emit!`, so it is
|
||||
;; not unit-testable, but the resolve step it gates on is.
|
||||
|
||||
(defn- inactive-set-library
|
||||
"A library with `primitives` (holding `color.gray.50`) left inactive and
|
||||
an active, empty `semantic` set — the repro from the issue."
|
||||
[]
|
||||
(-> (ctob/make-tokens-lib)
|
||||
(ctob/add-set (ctob/make-token-set :id (cthi/new-id! :primitives)
|
||||
:name "primitives"))
|
||||
(ctob/add-set (ctob/make-token-set :id (cthi/new-id! :semantic)
|
||||
:name "semantic"))
|
||||
(ctob/add-token (cthi/id :primitives)
|
||||
(ctob/make-token {:name "color.gray.50"
|
||||
:value "#fafafa"
|
||||
:type :color}))
|
||||
;; `add-set` does not activate sets, so activate only `semantic`,
|
||||
;; leaving `primitives` (the reference target) inactive.
|
||||
(ctob/toggle-set-in-theme ctob/hidden-theme-id "semantic")))
|
||||
|
||||
(t/deftest add-token-active-sets-only-fails-to-resolve-cross-set-reference
|
||||
;; Demonstrates the bug: resolving the new token against active sets
|
||||
;; only leaves the reference unresolved.
|
||||
(let [tokens-lib (inactive-set-library)
|
||||
token (ctob/make-token {:name "color.bg.default"
|
||||
:value "{color.gray.50}"
|
||||
:type :color})
|
||||
tokens-tree (-> (ctob/get-tokens-in-active-sets tokens-lib)
|
||||
(assoc (:name token) token))
|
||||
resolved (ts/resolve-tokens tokens-tree)
|
||||
{:keys [errors resolved-value]} (get resolved (:name token))]
|
||||
(t/is (nil? resolved-value))
|
||||
(t/is (seq errors))))
|
||||
|
||||
(t/deftest add-token-resolves-cross-set-reference-into-inactive-set
|
||||
;; The fix: resolving against all tokens in the library (inactive sets
|
||||
;; included) resolves the reference even though `primitives` is inactive.
|
||||
(let [tokens-lib (inactive-set-library)
|
||||
token (ctob/make-token {:name "color.bg.default"
|
||||
:value "{color.gray.50}"
|
||||
:type :color})
|
||||
tokens-tree (-> (merge (ctob/get-all-tokens-map tokens-lib)
|
||||
(ctob/get-tokens tokens-lib (cthi/id :semantic)))
|
||||
(assoc (:name token) token))
|
||||
resolved (ts/resolve-tokens tokens-tree)
|
||||
{:keys [errors resolved-value]} (get resolved (:name token))]
|
||||
(t/is (some? resolved-value))
|
||||
(t/is (empty? errors))))
|
||||
|
||||
(t/deftest add-token-still-fails-for-references-missing-from-every-set
|
||||
;; A reference to a token that exists in *no* set must still fail, even
|
||||
;; with the all-tokens resolution.
|
||||
(let [tokens-lib (inactive-set-library)
|
||||
token (ctob/make-token {:name "color.bg.default"
|
||||
:value "{color.does.not.exist}"
|
||||
:type :color})
|
||||
tokens-tree (-> (merge (ctob/get-all-tokens-map tokens-lib)
|
||||
(ctob/get-tokens tokens-lib (cthi/id :semantic)))
|
||||
(assoc (:name token) token))
|
||||
resolved (ts/resolve-tokens tokens-tree)
|
||||
{:keys [errors resolved-value]} (get resolved (:name token))]
|
||||
(t/is (nil? resolved-value))
|
||||
(t/is (seq errors))))
|
||||
|
||||
@ -13,6 +13,7 @@
|
||||
[app.common.types.text :as txt]
|
||||
[app.common.types.tokens-lib :as ctob]
|
||||
[app.main.data.workspace.tokens.application :as dwta]
|
||||
[app.main.data.workspace.tokens.library-edit :as dwtl]
|
||||
[app.main.data.workspace.wasm-text :as dwwt]
|
||||
[cljs.test :as t :include-macros true]
|
||||
[cuerdas.core :as str]
|
||||
@ -62,6 +63,53 @@
|
||||
(def debounce-text-stop
|
||||
(tohs/stop-on ::dwwt/resize-wasm-text-debounce-commit))
|
||||
|
||||
;; Regression coverage for issue #10070 (set-creation activation).
|
||||
;;
|
||||
;; Newly created token sets are inactive by default — only active sets
|
||||
;; affect shapes and reference resolution. The Plugin API's
|
||||
;; `addSet({ name, active })` creates an already-active set by emitting
|
||||
;; `create-token-set` followed by `set-enabled-token-set`. These tests
|
||||
;; pin that the create-then-enable sequence the proxy relies on actually
|
||||
;; ends with the set active (enabling only adds the set name to the hidden
|
||||
;; theme, so it does not depend on the create event having propagated).
|
||||
|
||||
(defn setup-file-with-empty-lib []
|
||||
(-> (setup-file)
|
||||
(assoc-in [:data :tokens-lib] (ctob/make-tokens-lib))))
|
||||
|
||||
(t/deftest test-create-token-set-inactive-by-default
|
||||
(t/testing "a newly created set is not active unless explicitly enabled"
|
||||
(t/async
|
||||
done
|
||||
(let [file (setup-file-with-empty-lib)
|
||||
store (ths/setup-store file)
|
||||
set (ctob/make-token-set :name "primitives")
|
||||
events [(dwtl/create-token-set set)]]
|
||||
(tohs/run-store-async
|
||||
store done events
|
||||
(fn [new-state]
|
||||
(let [file' (ths/get-file-from-state new-state)
|
||||
lib (get-in file' [:data :tokens-lib])]
|
||||
(t/is (some? (ctob/get-set lib (ctob/get-id set))))
|
||||
(t/is (false? (ctob/token-set-active? lib "primitives"))))))))))
|
||||
|
||||
(t/deftest test-create-then-enable-token-set
|
||||
(t/testing "create followed by set-enabled (as the plugin addSet does) yields an active set"
|
||||
(t/async
|
||||
done
|
||||
(let [file (setup-file-with-empty-lib)
|
||||
store (ths/setup-store file)
|
||||
set (ctob/make-token-set :name "primitives")
|
||||
events [(dwtl/create-token-set set)
|
||||
(dwtl/set-enabled-token-set "primitives" true)]]
|
||||
(tohs/run-store-async
|
||||
store done events
|
||||
(fn [new-state]
|
||||
(let [file' (ths/get-file-from-state new-state)
|
||||
lib (get-in file' [:data :tokens-lib])]
|
||||
(t/is (some? (ctob/get-set lib (ctob/get-id set))))
|
||||
(t/is (true? (ctob/token-set-active? lib "primitives"))))))))))
|
||||
|
||||
(t/deftest test-apply-token
|
||||
(t/testing "applies token to shape and updates shape attributes to resolved value"
|
||||
(t/async
|
||||
|
||||
@ -14,6 +14,8 @@
|
||||
- **plugin-types**: Added flag `throwValidationErrors` to enable exceptions on validation
|
||||
- **plugin-types**: Fix missing `webp` export format in `Export.type`
|
||||
- **plugin-types**: Added `fixedWhenScrolling` property for shapes
|
||||
- **plugin-runtime:** `addToken` now resolves references against all token sets, allowing references to tokens in inactive sets
|
||||
- **plugin-types:** `TokenCatalog.addSet` now accepts an optional `active` flag to create an already-active set (sets are inactive by default)
|
||||
|
||||
## 1.4.2 (2026-01-21)
|
||||
|
||||
|
||||
9
plugins/libs/plugin-types/index.d.ts
vendored
9
plugins/libs/plugin-types/index.d.ts
vendored
@ -5137,11 +5137,18 @@ export interface TokenCatalog {
|
||||
|
||||
/**
|
||||
* Creates a new TokenSet and adds it to the catalog.
|
||||
*
|
||||
* Newly created sets are **inactive** by default: only active sets
|
||||
* affect shapes and reference resolution. Pass `active: true` to create
|
||||
* an already-active set, or activate it later via `set.active = true` /
|
||||
* `set.toggleActive()`.
|
||||
* @param name The name of the set (required). It may contain
|
||||
* a group path, separated by `/`.
|
||||
* @param active Whether the set should be activated on creation.
|
||||
* Defaults to `false`.
|
||||
* @return Returns the created TokenSet.
|
||||
*/
|
||||
addSet({ name }: { name: string }): TokenSet;
|
||||
addSet({ name, active }: { name: string; active?: boolean }): TokenSet;
|
||||
|
||||
/**
|
||||
* Retrieves a theme.
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user