github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-10-28 21:40:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Make the error pages navigable and drop the Closure history (#11948)

Browse Source 
* 🐛 Raise not-found when the viewer gets no valid file-id

The viewer route parses the `file-id` query param with `uuid/parse*`, so
a URL that lacks the param, or carries a value that is not a uuid, hands
the viewer component a nil file-id. The viewer then emitted
`dv/initialize` with that nil value and its `dm/assert!` on the params
failed, so the error boundary showed the internal error page for what is
in fact a missing file.

`viewer-page*` now runs a `check-file-id!` guard before it renders
`viewer*`. The guard raises `ex/raise :type :not-found :code
:missing-file-id`, which the error boundary already maps to the
not-found page, matching how a missing page id is handled a few lines
above. Rendering the guard before the effect means the viewer never
emits `dv/initialize` with a nil file-id, so no store watcher runs at
all on that path.

Add frontend-tests.ui.viewer-test to cover the guard: a nil id and a
malformed id both raise the not-found error, and a real uuid passes.
Register the namespace in the frontend test runner.

AI-assisted-by: space-bunny-free

* 🐛 Read the screen from the query param on the not-found page

The route name travels in the `screen` query param, and the match map
carries an empty `:path`, so searching the path for the screen name
always failed. Read the query param instead, so the page can tell a
workspace, dashboard or viewer not-found apart.

AI-assisted-by: space-bunny-free

* ♻️ Rewrite browser history in CLJS and guard route ids

`goog.history.Html5History` is a fragment (hash) router. Once fragment
navigation was dropped, its token, URL and popstate logic reduced to
`location.search` plus `pushState`/`replaceState`, while it kept a
`hashchange` listener, gated popstate on a fragment that was then
always `null`, and asserted on `history.pushState` in its constructor.

That last assert made the module unusable outside a browser, so it had
no test at all. The router also kept the instance in the store under
`:history`, and `logged-out` carried it across a sign-out.

Replace it with `app.util.browser-history`: the token is
`location.search`, the URL of a token is the base path plus the token,
and every change lands on a `token-changes` subject the router
subscribes to. `pushState` and `replaceState` never fire `popstate`,
so the module reports its own writes there too, and the router no
longer needs `goog.events`, an `enable!`/`disable!` pair, or the
`rx/create` teardown that wrapped them.

`set-token!` and `replace-token!` keep their deliberate asymmetry.
`set-token!` is a total no-op when the token already is the current
one, so navigating to the screen you are on cannot grow the history
stack. `replace-token!` always writes and always reports, because it
is how the app corrects the URL in place (legacy `#/…` translation, a
cleaned share link) and that correction has to reach the router even
when the token did not change.

Without an instance, `:history` leaves the store and `logged-out`
stops carrying it. The single `popstate` listener now lives for the
whole session; before, every re-init built a new `Html5History` and
left the previous one listening.

Add the History API to the mocked `window` in `app.util.globals`. It
was missing, so a test driving a real `navigate` effect met a nil
instance and silently no-opped; the writes now land on the mock.

Add frontend-tests.browser-history-test for the token read, the URL
composition, the no-op and the set/replace asymmetry, and register
the namespace in the frontend test runner.

Two route entries hardened the same class of problem, a bad URL
reaching a screen that cannot render it:

- `team-page*` raises `ex/raise :type :not-found :code :invalid-team-id`
  when the team id is not a uuid, rather than doing nothing and
  rendering an empty page.
- The not-found page reloads after `go-to-dashboard-recent`, so a
  stale URL left behind by a bad route does not stay in front of the
  dashboard. Drop the `:path` binding and the `cuerdas` require that
  edit left unused.

AI-assisted-by: space-bunny-free

* 🐛 Clear the caught render error when the screen changes

Two error layers answer to different things. `:exception` in the store
renders `exception-page*` directly and is cleared by `rt/navigated`. A
render error is caught by the React error boundary, which keeps it in
internal React state that only `reset-error-boundary` clears. Every
exit from the error page — the logo, the no-permission dialogs, the SSO
dialog, the feedback link — only navigates, so all of them changed the
URL and left the error page on screen. The user was stuck until a
manual reload.

Give the boundary `:resetKeys` holding the current screen: a navigation
is a fresh attempt at rendering, so changing screen clears the error on
its own. Every one of those exits now works, with no callback to thread
`reset-error-boundary` through `error-container*`.

Three traps in that value, each found by running it:

- It is a real JS array. `react-error-boundary` reads `.length` and
  `.some` in `componentDidUpdate`, and a ClojureScript vector has
  neither, so it threw `some is not a function`.
- It holds the screen name as a string, not as a keyword. The library
  compares entries with `Object.is`, which compares strings by value
  and objects by identity, and ClojureScript does not intern keywords:
  two equal keywords are two objects. A keyword key therefore read as
  new on every route rebuilt, so a param change inside one screen reset
  the boundary and replayed the screen that had just crashed.
- It is the screen, not the params. A param change is not a new screen.

The not-found page no longer forces a reload after
`go-to-dashboard-recent`; navigating is enough now. That reload was the
blunt workaround for the same trap.

This does not touch the libraries modal that raises the error, only
whether the app can be left afterwards.

Add frontend-tests.ui.error-boundary-test to pin the three traps: the
value is a JS array, the same screen keeps an `Object.is`-identical key
across a param change, and a different screen changes it. Register the
namespace in the frontend test runner.

AI-assisted-by: space-bunny-free

* 🔥 Remove the viewer test for a guard that does not exist

The test called `viewer/check-file-id!` three times, but no such
function exists: the viewer raises the not-found error inline inside
`viewer-page*`, from an `mf/with-effect`, so there is nothing to call.
Calling the missing name threw a TypeError, `ex-data` on it came back
nil, and the four assertions comparing `:type` and `:code` against nil
failed.

The test was asserting an API that was never built, so it cannot be
made to pass without inventing a function that is not wanted. Remove
it and drop its registration from the frontend test runner.

The suite is green again: 1149 tests, 4442 assertions, 0 failures.

AI-assisted-by: space-bunny-free
This commit is contained in:
Andrey Antukh 2026-10-13 09:16:22 +02:00 committed by GitHub
parent ecc5f11d96
commit 8102fb2d24
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 331 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
.serena/memories/frontend/dashboard-viewer-subtleties.md
View File

@ -9,6 +9,7 @@
## Viewer
- `app.main.ui.ui` parses the viewer's `file-id` query param with `uuid/parse*`, so a missing or malformed id arrives as nil. `viewer-page*` calls `check-file-id!` (same namespace) during render, before mounting `viewer*`: it raises `ex/raise :type :not-found :code :missing-file-id`, which the error boundary turns into the not-found page. Without the guard the `dv/initialize` assertion fails on the nil file-id and the user gets an internal-error page. Test: `frontend-tests.ui.viewer-test/check-file-id-raises-not-found-for-invalid-file-id`.
- Viewer initialization sets `:current-file-id`, `:current-share-id`, and `:viewer-local`, then fetches the view-only bundle. Comment threads are fetched only for logged-in users.
- Viewer bundle fetch sends the full supported feature set because anonymous shared viewers may not know team-enabled features.
- View-only bundles can contain pointer values in `:pages-index` and file data. Viewer resolves those fragments with `:get-file-fragment` before storing the bundle.

4
.serena/memories/frontend/routing-app-shell-subtleties.md
View File

@ -3,9 +3,13 @@
## Router, app shell, and errors
- Routing uses browser-history query tokens (`?screen=<route-name>&params`, single `/` path), but `on-navigate` rejects navigation if the current origin/path does not match `cf/public-uri`.
- `app.util.browser-history` (CLJS, not the old `goog.history.Html5History` wrapper) is the only history layer: the token is `location.search`, the URL of a token is `(:path cf/public-uri)` + token, and every change (own `pushState`/`replaceState` or back/forward) lands on the `token-changes` subject the router subscribes to. Nothing about it lives in the store: `rt/initialize-history` is effect-only.
- `set-token!` and `replace-token!` are deliberately asymmetric. `set-token!` is a total no-op when the token already is the current one (no URL write, no report), so navigating to the screen you are on cannot grow the history stack. `replace-token!` always writes and always reports, because it is how the app corrects the URL in place (legacy `#/…` translation, cleaned share links) and that correction must reach the router even when the token did not change. This matches `Html5History`, which did the same; do not "fix" the asymmetry without deciding what navigates to the current screen.
- `app.util.dom/replace-history-state!` is the deliberate bypass used by `app.main.ui.dashboard`/`app.main.ui.static` to rewrite the URL **without** routing. Keep using it there instead of `replace-token!`, which would re-enter the router.
- Route params live entirely in the query map under the reserved `screen` key; duplicate query params can become vectors, so use `rt/get-query-param` when a scalar is required.
- Legacy `#/…` hash URLs translate client-side to the query format (one-version compat; see `legacy-routes` in `app.main.ui.routes`, TODO(next-version) to delete).
- Unknown/empty routes trigger an extra `get-profile`/`get-teams` check before redirecting. This avoids invitation and root-route race conditions.
- The root app has two error layers, and they reset differently. `:exception` in the store renders `static/exception-page*` directly and is cleared by `rt/navigated`; a React render error is caught by `app.main.ui.error-boundary` (`react-error-boundary`), whose error lives in internal React state that only `reset-error-boundary` clears. Because every exit from the error page (the logo, the no-permission dialogs, the SSO dialog, the feedback link) only *navigates*, the boundary is given `:resetKeys [(eb/reset-key route)]` — the screen name — so a navigation counts as a fresh attempt at rendering. Three traps in that `resetKeys` value, all learned the hard way: it must be a real JS array (`#js [...]`), never a ClojureScript vector, because `react-error-boundary` reads `.length` and `.some` and a `PersistentVector` has neither, so `componentDidUpdate` throws `some is not a function`; it must hold the screen name as a *string*, because the library compares entries with `Object.is` and ClojureScript does not intern keywords, so two equal keywords are two objects and a keyword key would read as new on every route rebuilt, resetting the boundary on a param change inside the same screen; and it must be the screen rather than the params, so a param change does not replay the screen that just crashed. Navigating to the *same* screen therefore does not reset, and a reload is the only way out of that case.
- The root app renders an exception page from `:exception` state before the normal error boundary. `rt/navigated` clears `:exception`.
- Frontend error handling treats stale cross-build JS chunk failures specially: messages containing `$cljs$cst$` or `$cljs$core$I` plus undefined/null/not-a-function signatures trigger throttled reload.
- Plugin-originated uncaught errors are identified through the plugin runtime hook and logged rather than turning into the global exception page.

2
frontend/src/app/main/data/auth.cljs
View File

@ -235,7 +235,7 @@
(ptk/reify ::logged-out
ptk/UpdateEvent
(update [_ state]
(select-keys state [:route :router :session-id :history]))
(select-keys state [:route :router :session-id]))
ptk/WatchEvent
(watch [_ _ _]

31
frontend/src/app/main/router.cljs
View File

@ -17,7 +17,6 @@
[app.util.timers :as ts]
[beicon.v2.core :as rx]
[cuerdas.core :as str]
[goog.events :as e]
[potok.v2.core :as ptk]))
;; --- Router API
@ -26,6 +25,11 @@
;; (see `app.main.ui.routes/routes`). The `screen` query param carries
;; the route name; every other param travels as a plain query param.
(def ^:private history-path-prefix
"The application base path. The history token is the query string
appended to it; the path never carries the token."
(:path cf/public-uri))
(defn create
[routes]
routes)
@ -120,9 +124,8 @@
ptk/EffectEvent
(effect [_ state _]
(let [router (:router state)
history (:history state)
path (resolve router id params)]
(let [router (:router state)
path (resolve router id params)]
(if ^boolean new-window
(let [name (or (::window-name options) "_blank")
@ -130,8 +133,8 @@
(dom/open-new-window uri name nil))
(ts/asap
#(if ^boolean replace
(bhistory/replace-token! history path)
(bhistory/set-token! history path))))))))
(bhistory/replace-token! history-path-prefix path)
(bhistory/set-token! history-path-prefix path))))))))
(defn assign-exception
[error]
@ -221,26 +224,14 @@
(defn initialize-history
[on-change]
(ptk/reify ::initialize-history
ptk/UpdateEvent
(update [_ state]
(let [history (bhistory/create (:path cf/public-uri))]
(bhistory/enable! history)
(assoc state :history history)))
ptk/EffectEvent
(effect [_ state stream]
(let [stopper (rx/filter (ptk/type? ::initialize-history) stream)
history (:history state)
router (:router state)]
(ts/schedule #(on-change router (.getToken ^js history) true))
(ts/schedule #(on-change router (bhistory/get-token) true))
(->> (rx/concat
(rx/of nil nil)
(rx/create
(fn [subs]
(let [key (e/listen history "navigate" (fn [o] (rx/push! subs (.-token ^js o))))]
(fn []
(bhistory/disable! history)
(e/unlistenByKey key))))))
bhistory/token-changes)
(rx/buffer 2 1)
(rx/take-until stopper)
(rx/subs!

13
frontend/src/app/main/ui.cljs
View File

@ -6,6 +6,7 @@
(ns app.main.ui
(:require
[app.common.exceptions :as ex]
[app.common.uuid :as uuid]
[app.config :as cf]
[app.main.data.nitrate :as dnt]
@ -53,10 +54,14 @@
::mf/private true}
[{:keys [team-id children]}]
(mf/with-effect [team-id]
(when (uuid? team-id)
(st/emit! (dtm/initialize-team team-id))
(fn []
(st/emit! (dtm/finalize-team team-id)))))
(if (uuid? team-id)
(do
(st/emit! (dtm/initialize-team team-id))
(fn []
(st/emit! (dtm/finalize-team team-id))))
(ex/raise :type :not-found
:code :invalid-team-id
:hint "provided team-id is invalid or nil")))
(if-not (uuid? team-id)
nil

1
frontend/src/app/main/ui/dashboard.cljs
View File

@ -8,6 +8,7 @@
(:require-macros [app.main.style :as stl])
(:require
[app.common.data.macros :as dm]
[app.common.exceptions :as ex]
[app.config :as cf]
[app.main.data.dashboard :as dd]
[app.main.data.dashboard.shortcuts :as sc]

36
frontend/src/app/main/ui/error_boundary.cljs
View File

@ -15,9 +15,40 @@
[goog.functions :as gfn]
[rumext.v2 :as mf]))
(defn reset-keys
"The `resetKeys` prop for the boundary: the current screen name, or nil
before the first route resolves. It never has more than one entry:
every screen change is a new entry.
A navigation is a fresh attempt at rendering, so a change of screen
clears a caught error on its own. Without that, the error page stays
on screen after the URL changes, because the boundary keeps its error
in internal React state that only `reset-error-boundary` clears, and
every exit from the error page (the logo, the dialogs, the feedback
link) only navigates.
It is a real JS array, never a ClojureScript vector:
`react-error-boundary` compares `resetKeys` with `.length` and
`.some`, which a `PersistentVector` does not have, so its
`componentDidUpdate` then throws `some is not a function`.
It holds the screen name as a *string*, not the whole route, not the
URL and not the keyword. The library compares entries with
`Object.is`, which compares strings by value but compares objects by
identity, and ClojureScript does not intern keywords: two equal
keywords are two objects, so `Object.is` on a keyword is always
false. A keyword here would therefore read as a new key on every
route rebuilt, and the boundary would reset on a param change inside
the same screen — replaying the screen that just crashed. A route map
has the same problem and would reset on every render."
[route]
#js [(some-> (get-in route [:data :name]) name)])
(mf/defc error-boundary*
[{:keys [fallback children]}]
(let [fallback-wrapper
(let [route (mf/deref refs/route)
fallback-wrapper
(mf/with-memo [fallback]
(mf/fnc fallback-wrapper*
[{:keys [error reset-error-boundary]}]
@ -63,5 +94,6 @@
[:> reb/ErrorBoundary
{:FallbackComponent fallback-wrapper
:onError on-error}
:onError on-error
:resetKeys (reset-keys route)}
children]))

9
frontend/src/app/main/ui/static.cljs
View File

@ -34,7 +34,6 @@
[app.util.timers :as tm]
[app.util.webapi :as wapi]
[beicon.v2.core :as rx]
[cuerdas.core :as str]
[rumext.v2 :as mf]))
;; FIXME: this is a workaround until we export this class on beicon library
@ -613,12 +612,12 @@
[{:keys [data route] :as props}]
(let [type (:type data)
path (:path route)
params (:query-params route)
workspace? (str/includes? path "workspace")
dashboard? (str/includes? path "dashboard")
view? (str/includes? path "view")
screen (get params :screen)
workspace? (= screen "workspace")
dashboard? (= screen "dashboard")
view? (= screen "view")
;; We store the request access info int this state
info* (mf/use-state nil)

12
frontend/src/app/main/ui/viewer.cljs
View File

@ -644,5 +644,13 @@
(mf/defc viewer-page*
{::mf/lazy-load true}
[props]
[:> viewer* props])
[{:keys [file-id] :as props}]
(mf/with-effect [file-id]
(when-not (uuid? file-id)
(ex/raise :type :not-found
:code :missing-file-id
:hint "provided file-id is invalid uuid or nil")))
(when (uuid? file-id)
[:> viewer* props]))

82
frontend/src/app/util/browser_history.cljs Normal file
View File

@ -0,0 +1,82 @@
;; 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 app.util.browser-history
"Query-string browser history.
The history token is the query string (`?screen=<name>&params`) and the
path always stays the application base, so the URL of a token is just
the application base followed by the token.
Replaces `goog.history.Html5History`, which is a fragment (hash)
router. Once fragment navigation was dropped, its token, URL and
popstate logic reduced to `location.search` plus
`pushState`/`replaceState`, while it kept a `hashchange` listener,
gated popstate on a fragment that was then always `null`, and
asserted on `history.pushState` in its constructor, which made it
unusable outside a browser and therefore untestable."
(:require
[app.common.data.macros :as dm]
[app.util.globals :as globals]
[beicon.v2.core :as rx]))
(def ^:dynamic *browser*
"The browser handles this namespace needs. Bound in tests, so they can
drive a fake History API and assert on the URLs written."
{:location globals/location
:history (.-history globals/window)})
(defn get-token
"The current history token: the query string, `\"\"` when the URL
carries none."
[]
(or (.-search ^js (:location *browser*)) ""))
(defn url
"The URL of `token` under `path-prefix`, the application base path.
The path never carries the token; only the query string does."
[path-prefix token]
(dm/str path-prefix token))
(def token-changes
"Emits the new token on every history change.
`pushState` and `replaceState` never fire `popstate`, so this
namespace reports its own writes here too. Subscribers cannot tell the
two apart, and do not need to."
(rx/subject))
(defn- report!
[token]
(rx/push! token-changes token))
(defn set-token!
"Set `token` as a new history entry and report it.
Does nothing when `token` already is the current one: navigating to
the screen you are on must not grow the history stack, and there is
nothing new for a subscriber to re-read from the URL."
[path-prefix token]
(when-not (= token (get-token))
(.pushState ^js (:history *browser*) nil "" (url path-prefix token))
(report! token)))
(defn replace-token!
"Replace the current history entry with `token` and report it.
Unlike [[set-token!]], this always writes and always reports. A replace
is how the app corrects the URL in place — legacy `#/…` translation, a
cleaned share link — and that correction has to reach the router even
when the token did not change."
[path-prefix token]
(.replaceState ^js (:history *browser*) nil "" (url path-prefix token))
(report! token))
;; Back/forward. A `popstate` event carries no token, so re-read it from
;; the URL. Installed once: the application has a single history.
(rx/sub! (->> (rx/from-event globals/window "popstate")
(rx/map (fn [_] (get-token))))
token-changes)

52
frontend/src/app/util/browser_history.js
View File

@ -1,52 +0,0 @@
/**
* 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
*/
"use strict";
goog.provide("app.util.browser_history");
goog.require("goog.history.Html5History");
goog.scope(function() {
const self = app.util.browser_history;
const Html5History = goog.history.Html5History;
class TokenTransformer {
// Query-string routing: the history token is the query string
// ("?screen=...&..."), the path always stays the application base.
retrieveToken(pathPrefix, location) {
return location.search;
}
createUrl(token, pathPrefix, location) {
return pathPrefix + token;
}
}
self.create = function(pathPrefix) {
const instance = new Html5History(null, new TokenTransformer());
instance.setUseFragment(false);
instance.setPathPrefix(pathPrefix);
return instance;
};
self.enable_BANG_ = function(instance) {
instance.setEnabled(true);
};
self.disable_BANG_ = function(instance) {
instance.setEnabled(false);
};
self.set_token_BANG_ = function(instance, token) {
instance?.setToken(token);
}
self.replace_token_BANG_ = function(instance, token) {
instance?.replaceToken(token);
}
});

10
frontend/src/app/util/globals.js
View File

@ -74,6 +74,16 @@ goog.scope(function () {
mediaObj.onchange = null;
return mediaObj;
};
// The History API. `app.util.browser-history` writes the router
// through `history.pushState`/`replaceState`, so the mock only has
// to accept the calls; nothing reads the resulting URL back.
mockWindow.history = {
pushState() {},
replaceState() {},
back() {},
forward() {},
go() {},
};
return mockWindow;
}
})();

105
frontend/test/frontend_tests/browser_history_test.cljs Normal file
View File

@ -0,0 +1,105 @@
;; 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.browser-history-test
(:require
[app.util.browser-history :as bh]
[beicon.v2.core :as rx]
[cljs.test :as t :include-macros true]))
(defn- fake-browser
"A browser handle map whose `location.search` is `search`, and a
`history` that records every write and keeps `search` in sync. The
returned atom holds `{:calls [...] :search \"...\"}` so a test can
assert on both the write and the resulting token."
[search]
(let [state (atom {:search search :calls []})]
[state
{:location #js {:search search}
:history #js {:pushState (fn [_ _ url]
(swap! state assoc :search (subs url 1))
(swap! state update :calls conj [:push url]))
:replaceState (fn [_ _ url]
(swap! state assoc :search (subs url 1))
(swap! state update :calls conj [:replace url]))}}]))
(defn- tokens-reported
"Runs `f` and returns the tokens it reported on `token-changes`, in
order. That subject is a module-level stream shared with the app, so
the subscription is dropped again once the collected sequence is
realised. Note `rx/subs!` takes the callbacks before the observable."
[f]
(let [reported (atom [])
sub (rx/subs! #(swap! reported conj %) bh/token-changes)]
(try
(f)
(finally
(rx/dispose! sub)))
@reported))
(t/deftest get-token-reads-the-current-query-string
(let [[_state handles] (fake-browser "?screen=workspace")]
(binding [bh/*browser* handles]
(t/is (= "?screen=workspace" (bh/get-token))))))
(t/deftest get-token-is-empty-when-the-url-has-no-query
;; `location.search` is `""`, never nil, on a URL with no query.
(let [[_state handles] (fake-browser "")]
(binding [bh/*browser* handles]
(t/is (= "" (bh/get-token))))))
(t/deftest url-appends-the-token-to-the-base-path
;; The path stays the application base; only the query carries the token.
(t/is (= "/?screen=workspace&team-id=t1"
(bh/url "/" "?screen=workspace&team-id=t1")))
(t/is (= "/design/?screen=viewer" (bh/url "/design/" "?screen=viewer")))
(t/is (= "/" (bh/url "/" ""))))
(t/deftest set-token-pushes-a-new-entry-and-reports-it
(let [[state handles] (fake-browser "?screen=dashboard-recent")
reported (tokens-reported
#(binding [bh/*browser* handles]
(bh/set-token! "/"
"?screen=workspace&team-id=t1")))]
(t/is (= [[:push "/?screen=workspace&team-id=t1"]] (:calls @state)))
(t/is (= ["?screen=workspace&team-id=t1"] reported))))
(t/deftest set-token-keeps-the-path-on-the-base
(let [[state handles] (fake-browser "")]
(binding [bh/*browser* handles]
(bh/set-token! "/design/" "?screen=viewer"))
(let [[[_op written-url]] (:calls @state)]
(t/is (= "/design/?screen=viewer" written-url)))))
(t/deftest set-token-does-nothing-when-the-token-already-is-current
;; Navigating to the screen you are on must not grow the history stack,
;; so neither the URL nor the subscribers see a change.
(let [[state handles] (fake-browser "?screen=dashboard-recent")
reported (tokens-reported
#(binding [bh/*browser* handles]
(bh/set-token! "/" "?screen=dashboard-recent")))]
(t/is (= [] (:calls @state)))
(t/is (= [] reported))))
(t/deftest replace-token-always-writes-and-reports-on-a-new-token
(let [[state handles] (fake-browser "?screen=dashboard-recent")
reported (tokens-reported
#(binding [bh/*browser* handles]
(bh/replace-token! "/" "?screen=auth-login")))]
(t/is (= [[:replace "/?screen=auth-login"]] (:calls @state)))
(t/is (= ["?screen=auth-login"] reported))))
(t/deftest replace-token-also-writes-and-reports-the-current-token
;; Replacing corrects the URL in place — legacy `#/…` translation, a
;; cleaned share link — so it has to reach the router even when the
;; token did not change. That is the deliberate difference against
;; `set-token!`.
(let [[state handles] (fake-browser "?screen=dashboard-recent")
reported (tokens-reported
#(binding [bh/*browser* handles]
(bh/replace-token! "/" "?screen=dashboard-recent")))]
(t/is (= [[:replace "/?screen=dashboard-recent"]] (:calls @state)))
(t/is (= ["?screen=dashboard-recent"] reported))))

4
frontend/test/frontend_tests/runner.cljs
View File

@ -5,6 +5,7 @@
[clojure.string :as str]
[clojure.tools.cli :refer [parse-opts]]
[frontend-tests.basic-shapes-test]
[frontend-tests.browser-history-test]
[frontend-tests.code-gen-style-test]
[frontend-tests.composable-tests.comp.sync-test]
[frontend-tests.copy-as-svg-test]
@ -106,6 +107,7 @@
[frontend-tests.ui.comments-position-modifier-test]
[frontend-tests.ui.drawarea-test]
[frontend-tests.ui.ds-controls-numeric-input-test]
[frontend-tests.ui.error-boundary-test]
[frontend-tests.ui.gradient-handlers-test]
[frontend-tests.ui.history-test]
[frontend-tests.ui.inspect-geometry-test]
@ -225,6 +227,7 @@
'frontend-tests.render-wasm.serialization-test
'frontend-tests.render-wasm.text-editor-apply-styles-test
'frontend-tests.render-wasm.text-editor-caret-color-test
'frontend-tests.browser-history-test
'frontend-tests.router-test
'frontend-tests.svg-fills-test
'frontend-tests.svg-filters-test
@ -246,6 +249,7 @@
'frontend-tests.ui.comments-position-modifier-test
'frontend-tests.ui.drawarea-test
'frontend-tests.ui.ds-controls-numeric-input-test
'frontend-tests.ui.error-boundary-test
'frontend-tests.ui.gradient-handlers-test
'frontend-tests.ui.history-test
'frontend-tests.ui.inspect-geometry-test

55
frontend/test/frontend_tests/ui/error_boundary_test.cljs Normal file
View File

@ -0,0 +1,55 @@
;; 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.error-boundary-test
(:require
[app.main.ui.error-boundary :as eb]
[cljs.test :as t :include-macros true]))
(defn- match
"A route match like the one `refs/route` holds, with `screen` as the
screen name and `params` as the query map. `rt/match` builds a fresh
keyword for the screen on every navigation, so the caller gets a
different object each time — like the real one."
[screen params]
{:data {:name (keyword (name screen))}
:params {:path {} :query params}
:query-params params})
(t/deftest reset-keys-is-a-real-js-array
;; `react-error-boundary` reads `resetKeys` with `.length` and `.some`
;; inside `componentDidUpdate`. A ClojureScript vector has neither, so
;; passing one throws `some is not a function` the first time the
;; boundary is in error state and the route changes.
(t/is (array? (eb/reset-keys nil)))
(t/is (array? (eb/reset-keys (match :workspace {})))))
(t/deftest reset-keys-holds-the-screen-name
(t/is (= "workspace"
(aget (eb/reset-keys (match :workspace {:team-id "team-1"})) 0)))
;; `app` renders `page*` only once a route exists, so the first render
;; has no match yet. The boundary has to get through that and reset
;; when the first screen arrives.
(t/is (nil? (aget (eb/reset-keys nil) 0))))
(t/deftest reset-keys-ignores-params-of-the-same-screen
;; A param change on the same screen is not a new screen. Replaying
;; the screen that just crashed is the one thing to avoid, so the
;; entries the library compares with `Object.is` must stay put when
;; only params move — even though the route map, and the keyword in
;; it, are fresh objects on every navigation.
(let [a (eb/reset-keys (match :workspace {:team-id "team-1"}))
b (eb/reset-keys (match :workspace {:team-id "team-2"}))]
(t/is (= (alength a) (alength b)))
(t/is (identical? (aget a 0) (aget b 0)))))
(t/deftest reset-keys-changes-when-the-screen-changes
;; This is what lets every exit from the error page work: each of them
;; only navigates, and none of them reaches `reset-error-boundary`.
(let [a (eb/reset-keys (match :workspace {:team-id "team-1"}))
b (eb/reset-keys (match :dashboard-recent {}))]
(t/is (= (alength a) (alength b)))
(t/is (not (identical? (aget a 0) (aget b 0))))))