fix(plugin-view,app-shell): refresh the view in place after a write instead of remounting it (objectui#10035)

.changeset/10035-objectview-refresh-in-place.md

@@ -0,0 +1,13 @@
+---
+'@object-ui/plugin-view': patch
+'@object-ui/app-shell': patch
+---
+
+A save, delete or other write no longer rebuilds the object view it happened in for most view types — the view refetches its rows in place and keeps its component state (objectui#10035).
+
+Both `ObjectView` layers carried their refresh counter in a React `key`, so every write remounted the whole view and reset everything held in it: a calendar jumped back to the current month and its default mode, the object page's list lost its scroll position and the visualization picked in its own switcher. The counter now reaches the rendered view as a data signal instead:
+
+- `@object-ui/app-shell`: the object page hands its refresh counter to `ListView` as `refreshTrigger`, the input `ListView`'s fetch already follows, and keys the list on the object and view alone.
+- `@object-ui/plugin-view`: kanban, calendar, gallery, timeline, map and tree views keep their instance across a write and receive the refetched rows.
+
+Views whose renderer fetches for itself and reads no refresh input are still remounted after a write, because that remount is the only way they show it: gantt and chart views in both layers, any object-page list whose visualization whitelist offers gantt or chart, and the standalone grid `plugin-view` renders when no host list view is supplied.

packages/app-shell/src/views/ObjectView.refreshInPlace-10035.test.tsx

@@ -0,0 +1,261 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * objectui#10035 — a write refreshes the list's DATA; it does not rebuild the
+ * list (AGENTS.md #8's corollary).
+ *
+ * ## The defect this pins
+ *
+ * `renderListView` keyed `<ListView>` on `objectName-viewId-counter`, where the
+ * counter is this page's refresh signal plus the plugin ObjectView's. The page
+ * bumps it after every write it hears about — record actions, import, realtime
+ * events, view edits, and the console's `externalRefreshKey` (record-form save,
+ * undo, redo). So each write REMOUNTED the list: its toolbar, search, popovers,
+ * selection and in-list visualization choice all went with it. The rows were
+ * refetched too — a remounted `ListView` fetches on mount — which is why
+ * nothing looked broken: the damage was the remount, not a missing refetch.
+ *
+ * ## The two halves, and which world each one can fail in
+ *
+ * The in-place cases assert BOTH:
+ *   (a) the list is the SAME mounted tree after the write — a DOM node read
+ *       inside `ListView` before the write is the node on the page after it
+ *       (a key remount replaces every node), and
+ *   (b) the list query was re-issued, exactly once.
+ * Against the pre-fix source (a) is the red half; (b) stays green there,
+ * because that world refetched as well — through the remount. (b) is the half
+ * that goes red if the counter leaves the key without reaching `ListView`
+ * another way (`refreshTrigger`), which is the regression a naive "remove the
+ * key" fix would ship: the page's own counter used to reach the list through
+ * the key alone.
+ *
+ * ## The remount that stays, pinned as the refresh it still is
+ *
+ * A list that can draw `gantt` or `chart` — as its own type, or through the
+ * author's visualization whitelist — keeps the counter in its key, because
+ * those renderers fetch for themselves and read nothing that moves on a write
+ * (`REMOUNT_TO_REFRESH_VISUALIZATIONS`). Those cases assert the fresh tree.
+ *
+ * Everything here renders for real — the page, `plugin-view`'s `ObjectView`
+ * and `plugin-list`'s `ListView` — on the harness of
+ * `ObjectView.hostRerenderRefetch-10046.test.tsx`.
+ */
+
+import * as React from 'react';
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { render, cleanup, act, screen } from '@testing-library/react';
+import { MemoryRouter, Routes, Route } from 'react-router-dom';
+
+vi.mock('@object-ui/permissions', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('@object-ui/permissions')>();
+  // STABLE identities: `ListView` names `perms` in its fetch dependency list,
+  // so a fresh object per call would loop the fetch on its own.
+  const perms = {
+    check: () => ({ allowed: true }),
+    checkField: () => true,
+    getFieldPermissions: () => [],
+    getRowFilter: () => undefined,
+    getObjectApiOperations: () => undefined,
+    roles: [],
+    isLoaded: false,
+    hasCapabilities: () => true,
+    can: () => true,
+    cannot: () => false,
+  };
+  const fieldPerms = { canRead: () => true, canWrite: () => true, permissions: [] };
+  return { ...actual, usePermissions: () => perms, useFieldPermissions: () => fieldPerms };
+});
+
+vi.mock('@object-ui/auth', async (importOriginal) => ({
+  ...(await importOriginal<Record<string, unknown>>()),
+  useAuth: () => ({ user: { id: 'u1', name: 'Ada' }, activeOrganization: null }),
+  useWorkspaceAdminStatus: () => ({ isAdmin: false, isResolved: true }),
+  createAuthenticatedFetch: () => vi.fn(),
+}));
+
+vi.mock('@object-ui/collaboration', async (importOriginal) => ({
+  ...(await importOriginal<Record<string, unknown>>()),
+  useRealtimeSubscription: () => ({ lastMessage: null }),
+  useConflictResolution: () => ({ hasConflicts: false, resolveAllConflicts: () => {} }),
+}));
+
+vi.mock('sonner', () => ({
+  toast: Object.assign(vi.fn(), {
+    success: vi.fn(), error: vi.fn(), info: vi.fn(),
+    warning: vi.fn(), loading: vi.fn(), dismiss: vi.fn(),
+  }),
+}));
+
+vi.mock('./MetadataInspector', () => ({
+  MetadataPanel: () => null,
+  useMetadataInspector: () => ({ showDebug: false, toggle: () => {} }),
+}));
+vi.mock('./RecordDetailView', () => ({ RecordDetailView: () => null }));
+
+import { ObjectView } from './ObjectView';
+import { ExpressionProvider } from '../providers/ExpressionProvider';
+
+const OBJECT_NAME = 'duly_task';
+
+/**
+ * The view under test is always `all`. Its `description` renders as
+ * `view-description` INSIDE `ListView`, which is the node whose identity (a)
+ * reads.
+ */
+function objectsWith(allView: Record<string, unknown>) {
+  return [
+    {
+      name: OBJECT_NAME,
+      label: 'Task',
+      fields: {
+        id: { type: 'text', label: 'Id' },
+        name: { type: 'text', label: 'Name' },
+        stage: { type: 'select', label: 'Stage', options: [{ label: 'A', value: 'a' }] },
+        starts_on: { type: 'date', label: 'Starts' },
+        ends_on: { type: 'date', label: 'Ends' },
+      },
+      listViews: {
+        all: { label: 'All', columns: ['name', 'stage'], description: 'Every task', ...allView },
+      },
+    },
+  ];
+}
+
+/** The list queries `ListView` issued; the page's `$top: 0` count probe is excluded. */
+let listQueries = 0;
+function makeDataSource() {
+  return {
+    find: vi.fn(async (_object: string, params: any) => {
+      if (params?.$top !== 0) listQueries++;
+      return { data: [], total: 0 };
+    }),
+    findOne: vi.fn(async () => null),
+    create: vi.fn(async () => ({})),
+    update: vi.fn(async () => ({})),
+    delete: vi.fn(async () => ({})),
+  } as any;
+}
+
+/** A window long enough for every effect a step schedules to have fired. */
+const settle = () => act(() => new Promise<void>((resolve) => setTimeout(resolve, 400)));
+
+/**
+ * Mounts the page and returns the console's write signal: `externalRefreshKey`
+ * is what `AppContent` bumps after a record-form save, an undo and a redo.
+ */
+async function mountPage(allView: Record<string, unknown>): Promise<{ write: () => Promise<void> }> {
+  const dataSource = makeDataSource();
+  const objects = objectsWith(allView);
+  let bump: () => void = () => {};
+  function Harness() {
+    const [externalRefreshKey, setExternalRefreshKey] = React.useState(0);
+    bump = () => setExternalRefreshKey((n) => n + 1);
+    return (
+      <ExpressionProvider user={{ id: 'u1', name: 'Ada', profile: 'admin' }}>
+        <MemoryRouter initialEntries={[`/apps/demo/${OBJECT_NAME}/view/all`]}>
+          <Routes>
+            <Route
+              path="/apps/:appName/:objectName/view/:viewId"
+              element={
+                <ObjectView
+                  dataSource={dataSource}
+                  objects={objects}
+                  onEdit={() => {}}
+                  externalRefreshKey={externalRefreshKey}
+                />
+              }
+            />
+          </Routes>
+        </MemoryRouter>
+      </ExpressionProvider>
+    );
+  }
+  render(<Harness />);
+  await settle();
+  return {
+    write: async () => {
+      await act(async () => bump());
+      await settle();
+    },
+  };
+}
+
+/** A node rendered inside `ListView` — replaced wholesale by a key remount. */
+const listNode = () => screen.getByTestId('view-description');
+
+beforeEach(() => {
+  cleanup();
+  listQueries = 0;
+  vi.stubGlobal(
+    'fetch',
+    vi.fn(async () =>
+      new Response(JSON.stringify({ data: [] }), {
+        status: 200,
+        headers: { 'content-type': 'application/json' },
+      }),
+    ),
+  );
+});
+
+afterEach(() => {
+  vi.unstubAllGlobals();
+  vi.clearAllMocks();
+});
+
+describe('the object page refreshes its list in place after a write (objectui#10035)', () => {
+  it.each([
+    ['grid', { type: 'grid' }],
+    ['kanban', { type: 'kanban', kanban: { groupByField: 'stage' } }],
+  ])('%s: the same list re-issues its query once', async (_label, allView) => {
+    const page = await mountPage(allView);
+    const node = listNode();
+    const before = listQueries;
+
+    await page.write();
+
+    expect(
+      listQueries - before,
+      '(b) The list did not re-query after the write, or re-queried more than once.\n'
+        + 'This page\'s refresh counter must reach `ListView` as `refreshTrigger` (the\n'
+        + 'input its fetch effect names) once it no longer rides in the key.',
+    ).toBe(1);
+    expect(
+      listNode(),
+      '(a) The write REMOUNTED the list: the node read inside `ListView` before the\n'
+        + 'write is gone. The refresh counter is back in the `<ListView>` key, so every\n'
+        + 'save throws the list\'s UI state away (AGENTS.md #8: refresh data, don\'t\n'
+        + 'rebuild UI). Only lists that can draw a `REMOUNT_TO_REFRESH_VISUALIZATIONS`\n'
+        + 'member may keep it.',
+    ).toBe(node);
+  });
+});
+
+describe('a list that can draw a view with no in-place refetch path still shows a write — by remount (objectui#10035)', () => {
+  it.each([
+    ['its own type is gantt', { type: 'gantt', gantt: { startDateField: 'starts_on', endDateField: 'ends_on' } }],
+    [
+      'gantt is in its visualization whitelist',
+      { type: 'grid', gantt: { startDateField: 'starts_on', endDateField: 'ends_on' }, appearance: { allowedVisualizations: ['grid', 'gantt'] } },
+    ],
+  ])('%s: a write mounts a fresh list', async (_label, allView) => {
+    const page = await mountPage(allView);
+    const node = listNode();
+
+    await page.write();
+
+    expect(
+      listNode(),
+      'The list was NOT remounted after a write although it can draw a gantt or chart.\n'
+        + 'Those renderers fetch for themselves and read no refresh input, so without the\n'
+        + 'remount they silently keep showing the pre-write rows. Remove a member from\n'
+        + '`REMOUNT_TO_REFRESH_VISUALIZATIONS` only together with a change that makes its\n'
+        + 'renderer refetch in place.',
+    ).not.toBe(node);
+  });
+});

packages/app-shell/src/views/ObjectView.tsx

@@ -503,6 +503,29 @@ function isSameOptionsValue(a: unknown, b: unknown, depth = 0): boolean {
     return false;
 }
 
+/**
+ * objectui#10035 — the visualizations `ListView` draws WITHOUT an in-place
+ * refetch path, so a list that can show one of them is still remounted to show
+ * a write (`remountKey` in `renderListView`).
+ *
+ * Every other visualization draws the rows `ListView` fetched, and `ListView`
+ * refetches in place on `refreshTrigger` — `tree` re-issues its own query when
+ * those rows change. These two query for themselves and read nothing that
+ * moves on a write:
+ *   - `gantt` — the registered `object-gantt` renderer hands `ObjectGantt` only
+ *     `schema` and `dataSource`; its own query names no refresh counter, no
+ *     `onMutation` and no invalidation bus.
+ *   - `chart` — `ObjectChart` runs its own aggregate query off the node and
+ *     reads no refresh input. (A view whose own type is `chart` never reaches
+ *     `ListView` on this page — it takes the chart branch — so this member is
+ *     about the in-list switcher.)
+ *
+ * ⛔ Do not drop a member to "finish" objectui#10035 — that turns a remount into
+ * a list that silently stops showing writes. A member leaves when its renderer
+ * refetches in place.
+ */
+const REMOUNT_TO_REFRESH_VISUALIZATIONS: ReadonlySet<string> = new Set(['gantt', 'chart']);
+
 /**
  * THE record-detail URL this list surface builds — one route shape, one place.
  *
@@ -2416,7 +2439,20 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an
     const renderListView = useCallback(({ schema: listSchema, dataSource: ds, onEdit: editHandler, className, refreshKey: pluginRefreshKey }: any) => {
         // Combine local refreshKey with the plugin ObjectView's refreshKey for full propagation
         const combinedRefreshKey = refreshKey + (pluginRefreshKey || 0);
-        const key = `${objectName}-${activeView.id}-${combinedRefreshKey}`;
+        // objectui#10035 — AGENTS.md #8's corollary: refresh data, don't
+        // rebuild UI. The counter above is a DATA signal, and it used to ride
+        // in the `key` of everything below, so every save, delete, import or
+        // realtime event threw the list away — its search box, open popovers,
+        // scroll, selection and in-list visualization choice with it.
+        // `identityKey` is what a remount is for (another object, another
+        // view); `ListView` now receives the counter as `refreshTrigger`, which
+        // its fetch effect already names, and refetches in place.
+        //
+        // `remountKey` is kept ONLY where the renderer has no in-place refetch
+        // path, so dropping the counter would make it silently stop showing
+        // writes (see `REMOUNT_TO_REFRESH_VISUALIZATIONS`).
+        const identityKey = `${objectName}-${activeView.id}`;
+        const remountKey = `${identityKey}-${combinedRefreshKey}`;
         const viewDef = activeView;
 
         // Per-user, per-view runtime-filter cache (advanced filter + search).
@@ -2473,14 +2509,18 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an
          */
         if (viewDef.type === 'chart') {
             const chartConfig = viewDef.chart || {};
+            // Both chart branches below keep `remountKey` (objectui#10035):
+            // `ObjectChart` runs its own query and reads no refresh input, so a
+            // remount is still the only way it shows a write.
+            //
             // ADR-0021 (#1890): dataset-bound chart — the single author-facing
             // shape. Selects dimensions/measures BY NAME and runs through the
             // governed queryDataset path (numbers consistent across surfaces).
             if (chartConfig.dataset) {
                 const dims: string[] = Array.isArray(chartConfig.dimensions) ? chartConfig.dimensions : [];
                 const vals: string[] = Array.isArray(chartConfig.values) ? chartConfig.values : [];
                 return (
-                    <Suspense key={key} fallback={<div className="p-4 text-sm text-muted-foreground">Loading chart…</div>}>
+                    <Suspense key={remountKey} fallback={<div className="p-4 text-sm text-muted-foreground">Loading chart…</div>}>
                         <ObjectChart
                             dataSource={ds}
                             schema={{
@@ -2511,7 +2551,7 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an
                     ? chartConfig.series
                     : [{ dataKey: valueField, label: valueField }];
             return (
-                <Suspense key={key} fallback={<div className="p-4 text-sm text-muted-foreground">Loading chart…</div>}>
+                <Suspense key={remountKey} fallback={<div className="p-4 text-sm text-muted-foreground">Loading chart…</div>}>
                     <ObjectChart
                         dataSource={ds}
                         schema={{
@@ -2564,6 +2604,13 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an
          */
         const fullSchema: ListViewSchema = {
             ...listSchema,
+            // objectui#10035 — the refresh signal, in place of the remount the
+            // key used to force. The spread above carries only the plugin
+            // ObjectView's counter; this page's own counter (actions, import,
+            // realtime, view edits, the console's `externalRefreshKey`) reached
+            // `ListView` through the key alone. Both counters only grow, so
+            // their sum moves whenever either does.
+            refreshTrigger: combinedRefreshKey,
             // The active view's display label (same string the ViewTabBar
             // shows) — ListView appends it to export download filenames.
             label: viewDef.label ?? listSchema.label,
@@ -2885,9 +2932,23 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an
             heldListOptions.current = fullSchema.options;
         }
 
+        // Every visualization this `ListView` can draw: its own `viewType` and,
+        // through the in-list switcher, the author's whitelist. If any of them
+        // cannot refetch in place, the list keeps `remountKey` (objectui#10035)
+        // — this host cannot see which one the switcher is showing.
+        const reachableVisualizations = [
+            fullSchema.viewType,
+            ...(fullSchema.appearance?.allowedVisualizations ?? []),
+        ];
+        const listKey = reachableVisualizations.some(
+            (v) => v != null && REMOUNT_TO_REFRESH_VISUALIZATIONS.has(v),
+        )
+            ? remountKey
+            : identityKey;
+
         return (
             <ListView
-                key={key}
+                key={listKey}
                 schema={fullSchema}
                 className={className}
                 onEdit={editHandler}