-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: use clientLoader + localStorage for resizable panel persistence #3386
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1d0a263
e99a9c4
ce22a0b
37f9fb6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| area: webapp | ||
| type: fix | ||
| --- | ||
|
|
||
| Upgrade Remix packages from 2.1.0 to 2.17.4 to address security vulnerabilities in React Router |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,7 +12,7 @@ import { | |
| StopCircleIcon, | ||
| } from "@heroicons/react/20/solid"; | ||
|
|
||
| import { useLoaderData, useRevalidator } from "@remix-run/react"; | ||
| import { type ClientLoaderFunctionArgs, useLoaderData, useRevalidator } from "@remix-run/react"; | ||
| import { type LoaderFunctionArgs, type SerializeFrom, json } from "@remix-run/server-runtime"; | ||
| import { type Virtualizer } from "@tanstack/react-virtual"; | ||
| import { | ||
|
|
@@ -95,7 +95,6 @@ import { RunEnvironmentMismatchError, RunPresenter } from "~/presenters/v3/RunPr | |
| import { clickhouseClient } from "~/services/clickhouseInstance.server"; | ||
| import { getImpersonationId } from "~/services/impersonation.server"; | ||
| import { logger } from "~/services/logger.server"; | ||
| import { getResizableSnapshot } from "~/services/resizablePanel.server"; | ||
| import { requireUserId } from "~/services/session.server"; | ||
| import { cn } from "~/utils/cn"; | ||
| import { lerp } from "~/utils/lerp"; | ||
|
|
@@ -279,10 +278,6 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => { | |
| throw error; | ||
| } | ||
|
|
||
| //resizable settings | ||
| const parent = await getResizableSnapshot(request, resizableSettings.parent.autosaveId); | ||
| const tree = await getResizableSnapshot(request, resizableSettings.tree.autosaveId); | ||
|
|
||
| const runsList = await getRunsListFromTableState({ | ||
| tableStateParam: url.searchParams.get("tableState"), | ||
| organizationSlug, | ||
|
|
@@ -297,13 +292,40 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => { | |
| trace: result.trace, | ||
| maximumLiveReloadingSetting: result.maximumLiveReloadingSetting, | ||
| resizable: { | ||
| parent, | ||
| tree, | ||
| parent: undefined as ResizableSnapshot | undefined, | ||
| tree: undefined as ResizableSnapshot | undefined, | ||
| }, | ||
| runsList, | ||
| }); | ||
| }; | ||
|
|
||
| function getLocalStorageSnapshot(key: string): ResizableSnapshot | undefined { | ||
| try { | ||
| const raw = localStorage.getItem(key); | ||
| if (raw) { | ||
| const parsed: unknown = JSON.parse(raw); | ||
| if (parsed != null && typeof parsed === "object" && "status" in parsed) { | ||
| return parsed as ResizableSnapshot; | ||
| } | ||
| } | ||
| } catch { | ||
| // Silently ignore localStorage errors | ||
| } | ||
| return undefined; | ||
| } | ||
|
|
||
| export async function clientLoader({ serverLoader }: ClientLoaderFunctionArgs) { | ||
| const serverData = await serverLoader<typeof loader>(); | ||
| return { | ||
| ...serverData, | ||
| resizable: { | ||
| parent: getLocalStorageSnapshot(resizableSettings.parent.autosaveId), | ||
| tree: getLocalStorageSnapshot(resizableSettings.tree.autosaveId), | ||
| }, | ||
| }; | ||
| } | ||
| clientLoader.hydrate = true as const; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚩 clientLoader.hydrate = true changes initial render behavior With Was this helpful? React with 👍 or 👎 to provide feedback.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good observation. One clarification: with The net effect is: panels flash briefly with default sizes → saved sizes on first SSR load, but on subsequent client-side navigations the saved sizes are available immediately. |
||
|
|
||
| type LoaderData = SerializeFrom<typeof loader>; | ||
|
|
||
| export default function Page() { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Prompts route missing clientLoader — resizable panel snapshots are always undefined
The PR removes the server-side
getResizableSnapshotcalls from the prompts route and replaces them withundefined, but unlike the runs route (route.tsx:317-327), noclientLoaderwas added to restore the values fromlocalStorage. This meansresizable.outer,resizable.vertical, andresizable.generationswill always beundefinedon the prompts page, so users' saved panel sizes will never be restored on page load — a regression from the previous behavior where they were read from cookies.Prompt for agents
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is intentional, not a regression. When the
snapshotprop isundefined, thereact-window-splitterlibrary reads from localStorage directly — see the library source atPanelGroupImpllines 300-313:Since
autosaveStrategydefaults to"localStorage"and the component already hasautosaveIdset, returningundefinedfrom the server lets the library's built-in localStorage persistence handle everything on the client.A
clientLoaderwas not used here because this route usestypedjson/useTypedLoaderDatafromremix-typedjson, which has its own serialization layer that doesn't compose cleanly withclientLoader(which routes data throughuseLoaderData).