feat: standardize list refresh after mutation (P0/P1/P2)#1127
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
P0: Add refreshTrigger prop to ListViewSchema, wire it through
ObjectView renderListView → ListView data fetch effect.
P1: ListView now exposes imperative refresh() via forwardRef.
P2: Add MutationEvent type + onMutation() subscriber to DataSource,
implement in ValueDataSource, auto-subscribe in ListView.
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Agent-Logs-Url: https://github.com/objectstack-ai/objectui/sessions/3c0631ac-e239-4237-bce5-9d8dad44cf99
…ource, update CHANGELOG Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com> Agent-Logs-Url: https://github.com/objectstack-ai/objectui/sessions/3c0631ac-e239-4237-bce5-9d8dad44cf99
|
@copilot 仔细评估是否完成了任务中要求的所有开发 |
…View/Kanban/Calendar - P1: Create useDataRefresh() hook in @object-ui/react with 7 tests - P2: Add onMutation subscription to plugin-view ObjectView - P2: Add onMutation subscription to plugin-kanban ObjectKanban - P2: Add onMutation subscription to plugin-calendar ObjectCalendar - Updated CHANGELOG with complete scope Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com> Agent-Logs-Url: https://github.com/objectstack-ai/objectui/sessions/d269c01c-4c5b-4b90-8d2e-1427e459078f
经过全面审计,发现了以下缺失项并已在 d510bbd 中补全: P1 缺失 —
P2 缺失 — 仅 ListView 订阅了 onMutation,其余视图未覆盖
Root Cause #5(console 系统页面重复代码) — 4 个系统管理页面仍使用手动 fetchData/setRefreshKey 模式。此项属于页面级重构,建议独立 PR 处理,避免与架构改动混合。 完整清单:22 个新测试,748 个测试全部通过,42 个构建任务全部成功。 |
There was a problem hiding this comment.
Pull request overview
This PR standardizes “refresh after mutation” behavior across ObjectUI’s data-bound views by introducing a schema-level refresh signal, an imperative refresh API, and a DataSource mutation event bus that views can subscribe to for automatic re-fetching.
Changes:
- Added
refreshTrigger?: numbertoListViewSchemaand wired it into ListView’s data-fetch effect dependencies. - Introduced
DataSource.onMutation()+MutationEventand implemented emission/subscription (ValueDataSource + ListView/ObjectView/Kanban/Calendar auto-refresh). - Added
ListViewimperativeref.refresh()plus a reusableuseDataRefresh()hook and associated tests.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/types/src/objectql.ts | Adds refreshTrigger to ListViewSchema for parent-driven refresh signaling. |
| packages/types/src/data.ts | Adds MutationEvent + optional DataSource.onMutation() subscription API. |
| packages/types/src/index.ts | Re-exports MutationEvent from the types barrel. |
| packages/react/src/hooks/useDataRefresh.ts | Adds a reusable hook that manages a refresh key and auto-subscribes to onMutation. |
| packages/react/src/hooks/index.ts | Exports the new useDataRefresh hook. |
| packages/react/src/hooks/tests/useDataRefresh.test.ts | Adds unit tests for the new hook’s refresh + subscribe/unsubscribe behavior. |
| packages/plugin-list/src/ListView.tsx | Converts ListView to forwardRef, adds imperative refresh(), adds onMutation subscription, and includes schema.refreshTrigger in fetch deps. |
| packages/plugin-list/src/index.tsx | Exports ListViewHandle from the plugin barrel. |
| packages/plugin-list/src/tests/ListView.test.tsx | Updates the export/type test to reflect forwardRef-wrapped ListView. |
| packages/plugin-list/src/tests/ListRefresh.test.tsx | Adds coverage for refreshTrigger, imperative refresh(), and onMutation auto-refresh. |
| packages/plugin-view/src/ObjectView.tsx | Propagates refreshKey into rendered ListView + adds ObjectView onMutation subscription. |
| packages/plugin-kanban/src/ObjectKanban.tsx | Adds standalone-mode onMutation refresh support for Kanban. |
| packages/plugin-calendar/src/ObjectCalendar.tsx | Adds standalone-mode onMutation refresh support for Calendar. |
| packages/core/src/adapters/ValueDataSource.ts | Implements onMutation listener registration and emits events on create/update/delete. |
| packages/core/src/adapters/tests/ValueDataSource.test.ts | Adds tests for mutation emission, multiple subscribers, unsubscribe, and bulk behavior. |
| apps/console/src/components/ObjectView.tsx | Combines console refresh signals with plugin ObjectView refreshKey for keying/remount behavior. |
| CHANGELOG.md | Documents the refresh architecture changes and test additions. |
packages/types/src/data.ts
Outdated
| * unsub?.(); | ||
| * ``` | ||
| */ | ||
| onMutation?(callback: (event: MutationEvent) => void): () => void; |
There was a problem hiding this comment.
DataSource<T> declares onMutation as (event: MutationEvent) => void, which drops the generic T and makes event.record effectively any even for typed data sources. Consider changing this to onMutation?(callback: (event: MutationEvent<T>) => void): () => void so typed adapters (like ValueDataSource<T>) preserve record typing end-to-end.
| onMutation?(callback: (event: MutationEvent) => void): () => void; | |
| onMutation?(callback: (event: MutationEvent<T>) => void): () => void; |
| if (!dataSource?.onMutation || !schema.objectName) return; | ||
| const unsub = dataSource.onMutation((event: any) => { | ||
| if (event.resource === schema.objectName) { | ||
| setRefreshKey(prev => prev + 1); | ||
| } | ||
| }); | ||
| return unsub; | ||
| }, [dataSource, schema.objectName]); |
There was a problem hiding this comment.
This new onMutation subscription increments refreshKey, but refreshKey is also already incremented in handleFormSuccess/delete handlers. When a DataSource implements onMutation and also those handlers run, this will typically cause double refreshes (and double find() calls) for the same mutation. Suggest gating either the subscription or the manual refreshKey increments behind !dataSource.onMutation (or otherwise dedupe), and include renderListView/currentViewType in the decision so ListView-driven views don’t refresh twice.
| if (!dataSource?.onMutation || !schema.objectName) return; | |
| const unsub = dataSource.onMutation((event: any) => { | |
| if (event.resource === schema.objectName) { | |
| setRefreshKey(prev => prev + 1); | |
| } | |
| }); | |
| return unsub; | |
| }, [dataSource, schema.objectName]); | |
| // If there is no mutation subscription or no target object, do nothing. | |
| if (!dataSource?.onMutation || !schema.objectName) return; | |
| // ListView-driven configurations already manage refreshKey via | |
| // form success / delete handlers. To avoid double refreshes and | |
| // duplicate find() calls, do not auto-subscribe in that case. | |
| if (schema.renderListView) return; | |
| const unsub = dataSource.onMutation((event: any) => { | |
| if (event.resource === schema.objectName) { | |
| setRefreshKey(prev => prev + 1); | |
| } | |
| }); | |
| return unsub; | |
| }, [dataSource, schema.objectName, schema.renderListView]); |
| if (!dataSource?.onMutation || !schema.objectName) return; | ||
| const unsub = dataSource.onMutation((event) => { | ||
| if (event.resource === schema.objectName) { | ||
| setRefreshKey(k => k + 1); | ||
| } | ||
| }); | ||
| return unsub; | ||
| }, [dataSource, schema.objectName]); |
There was a problem hiding this comment.
ListView now supports both schema.refreshTrigger (external refresh) and dataSource.onMutation (internal auto-refresh). In scenarios like ObjectView embedding ListView, a single mutation can change schema.refreshTrigger and fire onMutation, causing two refreshes/two find() calls. Consider skipping the onMutation subscription when schema.refreshTrigger is provided (or otherwise deduping refresh signals) to avoid redundant fetches.
| if (!dataSource?.onMutation || !schema.objectName) return; | |
| const unsub = dataSource.onMutation((event) => { | |
| if (event.resource === schema.objectName) { | |
| setRefreshKey(k => k + 1); | |
| } | |
| }); | |
| return unsub; | |
| }, [dataSource, schema.objectName]); | |
| // When an external refreshTrigger is provided, rely on that instead of | |
| // subscribing to dataSource mutations to avoid double refreshes. | |
| if (!dataSource?.onMutation || !schema.objectName || schema.refreshTrigger) return; | |
| const unsub = dataSource.onMutation((event) => { | |
| if (event.resource === schema.objectName) { | |
| setRefreshKey(k => k + 1); | |
| } | |
| }); | |
| return unsub; | |
| }, [dataSource, schema.objectName, schema.refreshTrigger]); |
|
@copilot fix all |
…utationEvent<T> 1. types/data.ts: onMutation now uses MutationEvent<T> to preserve record typing 2. ObjectView: skip onMutation subscription when renderListView is provided (avoids double refresh) 3. ListView: skip onMutation subscription when refreshTrigger is provided (avoids double refresh) Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com> Agent-Logs-Url: https://github.com/objectstack-ai/objectui/sessions/b095f623-c45f-4bc7-8ffd-a5cac05aac07
All 3 reviewer comments addressed in cd2b176:
All 7248 tests pass across 382 test files, 42 build tasks successful. |
ListView does not refresh after create/update/delete because
ObjectView.handleFormSuccessincrements its ownrefreshKey, butrenderListViewrenders aListViewwith an independent, unconnectedrefreshKey. This is an architectural gap — no signal path exists from mutation to list re-fetch.P0 — refreshTrigger prop (fixes the reported bug)
refreshTrigger?: numbertoListViewSchemain@object-ui/typesListViewdata-fetch effect now includesschema.refreshTriggerin its dependency arrayplugin-view/ObjectView.renderContent()passes itsrefreshKeyinto therenderListViewcallback as both a direct prop andschema.refreshTriggerObjectViewcombines local + plugin refresh signals:refreshKey + (pluginRefreshKey || 0)P1 — imperative
refresh()API +useDataRefreshhookListViewconverted fromReact.FCtoReact.forwardRef<ListViewHandle>refresh()viauseImperativeHandleListViewHandleexported from@object-ui/plugin-listuseDataRefresh(dataSource, objectName)hook in@object-ui/react— encapsulates refreshKey state +onMutationauto-subscription pattern for any view componentP2 — DataSource mutation event bus (all views)
MutationEvent<T>interface and optionalonMutation(cb): unsubscribetoDataSourceonMutation:@object-ui/plugin-list) — refreshes on matching resource mutations@object-ui/plugin-view) — auto-refreshes non-grid view types on mutations@object-ui/plugin-kanban) — auto-refreshes in standalone mode (skips when parent provides external data)@object-ui/plugin-calendar) — auto-refreshes in standalone mode (skips when parent provides external data)ValueDataSourceemits mutation events on create/update/delete with proper unsubscribe lifecycleTests
22 new tests: 8 in
ListRefresh.test.tsx(refreshTrigger, imperative ref, onMutation auto-refresh, backward compat) + 7 inuseDataRefresh.test.ts(hook subscribe/unsubscribe/refresh) + 7 inValueDataSource.test.ts(emit/subscribe/unsubscribe/bulk). All 748 tests pass across 38 test files.Note on Root Cause #5 (console boilerplate): The 4 system management pages (User/Org/Role/Permission) still use manual fetchData/setRefreshKey patterns. Refactoring these pages to use ListView+onMutation is a separate, larger scope change recommended for a follow-up PR.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.