From 15157c54b1315b487bdf9498fe400cd358fdb7a5 Mon Sep 17 00:00:00 2001
From: Pablo Alba <pablo.alba@kaleidos.net>
Date: Wed, 22 Jan 2025 16:05:50 +0100
Subject: [PATCH] :bug: Fix shape-ref cycles
---
CHANGES.md | 1 +
common/src/app/common/files/repair.cljc | 29 ++++++++++++++++
common/src/app/common/files/validate.cljc | 18 ++++++++--
common/src/app/common/types/file.cljc | 40 +++++++++++++----------
4 files changed, 68 insertions(+), 20 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index b3fc1a750..79da857ae 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -6,6 +6,7 @@
- Fix detach when top copy is dangling and nested copy is not [Taiga #9699](https://tree.taiga.io/project/penpot/issue/9699)
- Fix problem in plugins with `replaceColor` method [#174](https://github.com/penpot/penpot-plugins/issues/174)
+- Fix issue with recursive commponents [Taiga #9903](https://tree.taiga.io/project/penpot/issue/9903)
- Fix missing methods reference on API Docs
diff --git a/common/src/app/common/files/repair.cljc b/common/src/app/common/files/repair.cljc
index 67f90dafe..5d80c844f 100644
--- a/common/src/app/common/files/repair.cljc
+++ b/common/src/app/common/files/repair.cljc
@@ -320,6 +320,35 @@
(pcb/with-file-data file-data)
(pcb/update-shapes shape-ids detach-shape))))))
+
+(defmethod repair-error :shape-ref-cycle
+ [_ {:keys [shape args] :as error} file-data _]
+ (let [repair-component
+ (fn [component]
+ (let [objects (:objects component) ;; we only have encounter this on deleted components,
+ ;; so the relevant objects are inside the component
+ to-detach (->> (:cycles-ids args)
+ (map #(get objects %))
+ (map #(ctn/get-head-shape objects %))
+ (map :id)
+ distinct
+ (mapcat #(ctn/get-children-in-instance objects %))
+ (map :id)
+ set)]
+
+ (update component :objects
+ (fn [objects]
+ (reduce-kv (fn [acc k v]
+ (if (contains? to-detach k)
+ (assoc acc k (ctk/detach-shape v))
+ (assoc acc k v)))
+ {}
+ objects)))))]
+ (log/dbg :hint "repairing component :shape-ref-cycle" :id (:id shape) :name (:name shape))
+ (-> (pcb/empty-changes nil nil)
+ (pcb/with-library-data file-data)
+ (pcb/update-component (:id shape) repair-component))))
+
(defmethod repair-error :shape-ref-in-main
[_ {:keys [shape page-id] :as error} file-data _]
(let [repair-shape
diff --git a/common/src/app/common/files/validate.cljc b/common/src/app/common/files/validate.cljc
index 79e6cf301..f1f2bdda9 100644
--- a/common/src/app/common/files/validate.cljc
+++ b/common/src/app/common/files/validate.cljc
@@ -55,7 +55,8 @@
:component-nil-objects-not-allowed
:instance-head-not-frame
:misplaced-slot
- :missing-slot})
+ :missing-slot
+ :shape-ref-cycle})
(def ^:private schema:error
[:map {:title "ValidationError"}
@@ -482,6 +483,18 @@
"This deleted component has children with the same swap slot"
component file nil))))
+(defn check-ref-cycles
+ [component file]
+ (let [cycles-ids (->> component
+ :objects
+ vals
+ (filter #(= (:id %) (:shape-ref %)))
+ (map :id))]
+
+ (when (seq cycles-ids)
+ (report-error :shape-ref-cycle
+ "This deleted component has shapes with shape-ref pointing to self"
+ component file nil :cycles-ids cycles-ids))))
(defn- check-component
"Validate semantic coherence of a component. Report all errors found."
@@ -491,7 +504,8 @@
"Objects list cannot be nil"
component file nil))
(when (:deleted component)
- (check-component-duplicate-swap-slot component file)))
+ (check-component-duplicate-swap-slot component file)
+ (check-ref-cycles component file)))
(defn- get-orphan-shapes
[{:keys [objects] :as page}]
diff --git a/common/src/app/common/types/file.cljc b/common/src/app/common/types/file.cljc
index 8236631ef..9b575179a 100644
--- a/common/src/app/common/types/file.cljc
+++ b/common/src/app/common/types/file.cljc
@@ -335,24 +335,28 @@
(true? (= (:id component) (:id ref-component)))))
(defn find-swap-slot
- [shape container file libraries]
- (if-let [swap-slot (ctk/get-swap-slot shape)]
- swap-slot
- (let [ref-shape (find-ref-shape file
- container
- libraries
- shape
- :include-deleted? true
- :with-context? true)
- shape-meta (meta ref-shape)
- ref-file (:file shape-meta)
- ref-container (:container shape-meta)]
- (when ref-shape
- (if-let [swap-slot (ctk/get-swap-slot ref-shape)]
- swap-slot
- (if (ctk/main-instance? ref-shape)
- (:id shape)
- (find-swap-slot ref-shape ref-container ref-file libraries)))))))
+ ([shape container file libraries]
+ (find-swap-slot shape container file libraries #{}))
+ ([shape container file libraries viewed-ids]
+ (if (contains? viewed-ids (:id shape)) ;; prevent cycles
+ nil
+ (if-let [swap-slot (ctk/get-swap-slot shape)]
+ swap-slot
+ (let [ref-shape (find-ref-shape file
+ container
+ libraries
+ shape
+ :include-deleted? true
+ :with-context? true)
+ shape-meta (meta ref-shape)
+ ref-file (:file shape-meta)
+ ref-container (:container shape-meta)]
+ (when ref-shape
+ (if-let [swap-slot (ctk/get-swap-slot ref-shape)]
+ swap-slot
+ (if (ctk/main-instance? ref-shape)
+ (:id shape)
+ (find-swap-slot ref-shape ref-container ref-file libraries (conj viewed-ids (:id shape)))))))))))
(defn match-swap-slot?
[shape-main shape-inst container-inst container-main file libraries]