mirror of
https://github.com/penpot/penpot.git
synced 2026-10-29 05:50:35 +00:00
8102fb2d24
* 🐛 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