mirror of
https://github.com/penpot/penpot.git
synced 2026-11-10 20:10:37 +00:00
🐛 Call the team hook on every render of the team container (#12920)
team-container* called mf/deref only after returning early for a team id that is not a uuid. When the same component rendered once without a team id and then with one, the number of hooks changed between the two renders and React threw "Rendered more hooks than during the previous render" (error #310), which sent the user to the internal error page. The reverse order throws "Rendered fewer hooks than expected". Call the hook on every render and keep the uuid check in the condition that builds the providers. Two tests mount the component in jsdom and render it with a team id that arrives and with one that leaves; both fail without this change. Closes #12728 AI-assisted-by: claude-opus-5-5 AI-assisted-by: claude-sonnet-5-5 Signed-off-by: Filip Sajdak <filip.sajdak@siili.com>
This commit is contained in:
parent
72575e5c05
commit
90d98b1989
@ -64,16 +64,14 @@
|
||||
:code :invalid-team-id
|
||||
:hint "provided team-id is invalid or nil")))
|
||||
|
||||
(if-not (uuid? team-id)
|
||||
nil
|
||||
(let [{:keys [permissions] :as team} (mf/deref refs/team)]
|
||||
(when (= team-id (:id team))
|
||||
[:> (mf/provider ctx/current-team-id) {:value team-id}
|
||||
[:> (mf/provider ctx/permissions) {:value permissions}
|
||||
[:> (mf/provider ctx/can-edit?) {:value (:can-edit permissions)}
|
||||
;; The `:key` is mandatory here because we want to reinitialize
|
||||
;; all dom tree instead of simple rerender.
|
||||
[:* {:key (str team-id)} children]]]]))))
|
||||
(let [{:keys [permissions] :as team} (mf/deref refs/team)]
|
||||
(when (and (uuid? team-id) (= team-id (:id team)))
|
||||
[:> (mf/provider ctx/current-team-id) {:value team-id}
|
||||
[:> (mf/provider ctx/permissions) {:value permissions}
|
||||
[:> (mf/provider ctx/can-edit?) {:value (:can-edit permissions)}
|
||||
;; The `:key` is mandatory here because we want to reinitialize
|
||||
;; all dom tree instead of simple rerender.
|
||||
[:* {:key (str team-id)} children]]]])))
|
||||
|
||||
(mf/defc page*
|
||||
{::mf/props :obj
|
||||
|
||||
@ -167,6 +167,7 @@
|
||||
[frontend-tests.ui.sortable-auto-scroll-test]
|
||||
[frontend-tests.ui.static-test]
|
||||
[frontend-tests.ui.stroke-menu-test]
|
||||
[frontend-tests.ui.team-container-test]
|
||||
[frontend-tests.ui.text-attrs-multiple-test]
|
||||
[frontend-tests.ui.viewport-top-bar-test]
|
||||
[frontend-tests.ui.workspace-libraries-test]
|
||||
@ -369,6 +370,7 @@
|
||||
'frontend-tests.ui.sortable-auto-scroll-test
|
||||
'frontend-tests.ui.static-test
|
||||
'frontend-tests.ui.stroke-menu-test
|
||||
'frontend-tests.ui.team-container-test
|
||||
'frontend-tests.ui.text-attrs-multiple-test
|
||||
'frontend-tests.ui.viewport-top-bar-test
|
||||
'frontend-tests.ui.workspace-libraries-test
|
||||
|
||||
80
frontend/test/frontend_tests/ui/team_container_test.cljs
Normal file
80
frontend/test/frontend_tests/ui/team_container_test.cljs
Normal file
@ -0,0 +1,80 @@
|
||||
;; 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 SUBSIDIARY SL
|
||||
|
||||
(ns frontend-tests.ui.team-container-test
|
||||
"`team-container*` renders the same hooks with and without a team id.
|
||||
React throws when the hook count changes between two renders of the
|
||||
same component, so these tests mount it and render it again; the
|
||||
documents come from jsdom since the runner has no DOM."
|
||||
(:require
|
||||
["jsdom" :refer [JSDOM]]
|
||||
["react-dom" :as rd]
|
||||
["react-dom/client" :as rdc]
|
||||
[app.common.uuid :as uuid]
|
||||
[app.main.refs :as refs]
|
||||
[app.main.ui :as ui]
|
||||
[cljs.test :as t :include-macros true]
|
||||
[cuerdas.core :as str]
|
||||
[okulary.core :as l]
|
||||
[rumext.v2 :as mf]))
|
||||
|
||||
(def ^:private team-id (uuid/next))
|
||||
|
||||
(defn- next-immediate
|
||||
"Resolves after the tasks React's scheduler queued with `setImmediate`
|
||||
so far have run."
|
||||
[]
|
||||
(js/Promise. (fn [resolve] (js/setImmediate resolve))))
|
||||
|
||||
(defn- ^:async render-with
|
||||
"Mounts `team-container*` once and renders it again with each of
|
||||
`team-ids`, in order. Resolves to the errors React reported and the
|
||||
markup left in the document.
|
||||
|
||||
react-dom reads the global `window` while it renders, and its scheduler
|
||||
does so again on a later turn, so the jsdom one stays installed until
|
||||
the root is unmounted and the scheduler has run.
|
||||
|
||||
The effect is replaced by a hook that does nothing: the real one raises
|
||||
for an invalid id, and what is under test is the render. It must still
|
||||
take a hook slot, because React only compares the hooks of two renders
|
||||
when the first one had any."
|
||||
[team-ids]
|
||||
(let [errors (volatile! [])
|
||||
on-error (fn [error _info] (vswap! errors conj error))
|
||||
window (.-window (JSDOM. "<!doctype html><div id=\"root\"></div>"))
|
||||
container (.getElementById (.-document window) "root")
|
||||
root (rdc/createRoot container #js {:onUncaughtError on-error
|
||||
:onCaughtError on-error
|
||||
:onRecoverableError on-error})
|
||||
child (mf/html [:span "inside"])
|
||||
previous (.-window js/globalThis)]
|
||||
(set! (.-window js/globalThis) window)
|
||||
(try
|
||||
(with-redefs [refs/team (l/atom {:id team-id :permissions {:can-edit true}})
|
||||
mf/useEffect (fn [_f _deps] (mf/use-ref nil))]
|
||||
(doseq [id team-ids]
|
||||
(rd/flushSync
|
||||
(fn []
|
||||
(.render root (mf/element (deref #'ui/team-container*)
|
||||
#js {:teamId id :children child}))))))
|
||||
(let [markup (.-innerHTML container)]
|
||||
(rd/flushSync #(.unmount root))
|
||||
(await (next-immediate))
|
||||
{:errors @errors
|
||||
:markup markup})
|
||||
(finally
|
||||
(set! (.-window js/globalThis) previous)))))
|
||||
|
||||
(t/deftest ^:async team-id-arriving-after-the-first-render-keeps-the-hooks
|
||||
(let [{:keys [errors markup]} (await (render-with [nil team-id]))]
|
||||
(t/is (empty? (map #(.-message %) errors)))
|
||||
(t/is (str/includes? markup "inside"))))
|
||||
|
||||
(t/deftest ^:async team-id-leaving-after-the-first-render-keeps-the-hooks
|
||||
(let [{:keys [errors markup]} (await (render-with [team-id nil]))]
|
||||
(t/is (empty? (map #(.-message %) errors)))
|
||||
(t/is (= "" markup))))
|
||||
Loading…
x
Reference in New Issue
Block a user