feat: 增加supabase可自定义同步数据#2
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds customizable Supabase cloud synchronization functionality to allow users to sync their personalized settings and data across devices using their own Supabase instances.
Key Changes:
- Implemented bidirectional cloud sync with configurable Supabase connection (URL, anon key, sync ID)
- Added auto-sync capability with debouncing (1.5s delay) for automatic data uploads on changes
- Refactored TodoWidget to support controlled component pattern for centralized state management
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pages/Index.jsx | Added cloud sync UI, state management for Supabase config and sync ID, auto-sync effects, and pull/push handlers for cloud synchronization |
| src/lib/cloudSync.js | New module providing cloud state sync functions (pull/push) with error handling and Supabase client creation |
| src/integrations/supabase/client.js | Replaced hardcoded credentials with dynamic configuration storage/retrieval and client ID generation utilities |
| src/components/TodoWidget.jsx | Refactored to support controlled component pattern with optional todos/onChange props while maintaining backward compatibility |
| public/manifest.json | Added Supabase domain permissions and CSP directives for cloud sync connectivity |
| README.md | Added Supabase setup documentation with SQL schema and RLS policy examples |
Comments suppressed due to low confidence (1)
README.md:34
- The RLS policies shown in the documentation (lines 29-34) allow unrestricted access with using (true) and with check (true), which means any anonymous user can read, insert, or update any row in the table. This contradicts the documentation comment on line 28 which mentions "只允许 client_id 匹配的读写策略" (only allow read/write policies where client_id matches). The policies should include proper client_id filtering to actually restrict access as documented.
-- 示例策略(匿名访问):如果只你自己使用该项目,可以放宽为 anon 全权限
create policy "anon read own state" on navinocode_states
for select using (true);
create policy "anon upsert state" on navinocode_states
for insert with check (true);
create policy "anon update state" on navinocode_states
for update using (true);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| table = DEFAULT_SYNC_TABLE, | ||
| syncId |
There was a problem hiding this comment.
The table parameter in pushCloudState has a default value but the parameter is in the middle position. This can cause confusion when calling the function. According to the function calls in Index.jsx (line 476), the syncId is passed as the fourth parameter while table uses its default. Consider reordering parameters to put optional parameters with defaults at the end, or using an options object pattern for better clarity.
| table = DEFAULT_SYNC_TABLE, | |
| syncId | |
| syncId, | |
| table = DEFAULT_SYNC_TABLE |
| const syncId = (supabaseSyncId || '').trim(); | ||
| if (!syncId) { | ||
| if (!options.silent) toast('请设置同步 ID'); | ||
| return; | ||
| } | ||
| setIsSyncing(true); | ||
| try { | ||
| const { payload, updatedAt } = await pullCloudState(supabaseConfig, undefined, syncId); |
There was a problem hiding this comment.
The validation logic checks if syncId is empty after trimming (line 429), but then passes the untrimmed supabaseSyncId to pullCloudState. This inconsistency could lead to issues if the syncId contains only whitespace. Use the trimmed syncId variable consistently throughout the function.
| const syncId = (supabaseSyncId || '').trim(); | ||
| if (!syncId) { | ||
| if (!options.silent) toast('请设置同步 ID'); | ||
| return; | ||
| } | ||
| setIsSyncing(true); | ||
| try { | ||
| const payload = { | ||
| apps, | ||
| todos, | ||
| componentSettings, | ||
| searchEngine, | ||
| backgroundImage, | ||
| backgroundBrightness, | ||
| backgroundBlur | ||
| }; | ||
| await pushCloudState(supabaseConfig, payload, undefined, syncId); |
There was a problem hiding this comment.
The validation logic checks if syncId is empty after trimming (line 460), but then passes the untrimmed supabaseSyncId to pushCloudState. This inconsistency could lead to issues if the syncId contains only whitespace. Use the trimmed syncId variable consistently throughout the function.
| if (!syncId) { | ||
| if (!options.silent) toast('请设置同步 ID'); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The isSyncing state is not reset to false when the early return conditions are met (lines 425-433). If validation fails, the function returns early and isSyncing remains true, potentially disabling the sync buttons permanently. Move setIsSyncing(true) to after all validation checks.
| } | |
| } | |
| if (!payload) { | ||
| if (!options.silent) toast('云端暂无数据,可先上传本地数据'); | ||
| return; | ||
| } | ||
| applyCloudPayload(payload); | ||
| if (updatedAt) { | ||
| setLastSyncedAt(updatedAt); | ||
| localStorage.setItem('lastSyncedAt', updatedAt); | ||
| } | ||
| if (!options.silent) toast('已从云端恢复数据'); | ||
| } catch (error) { | ||
| console.error('拉取云端数据失败', error); | ||
| if (!options.silent) toast(`拉取失败:${error.message || '未知错误'}`); | ||
| } finally { | ||
| setIsSyncing(false); |
There was a problem hiding this comment.
The setIsSyncing(false) in the finally block (line 451) will not execute if there's an early return before the try block (lines 438-439). This can leave isSyncing stuck at true if the cloud returns no data. Consider restructuring to ensure isSyncing is always reset, or move the early return inside the try-finally.
| onClick={handlePullFromCloud} | ||
| disabled={isSyncing} | ||
| > | ||
| <RefreshCcw className="h-4 w-4 mr-2" /> | ||
| 从云端拉取 | ||
| </Button> | ||
| <Button | ||
| className="rounded-2xl" | ||
| onClick={handlePushToCloud} |
There was a problem hiding this comment.
The handlePullFromCloud and handlePushToCloud functions are called directly as onClick handlers without wrapping them to prevent the default event parameter from being passed as the options argument. While this might work currently, it could cause unexpected behavior if the functions are refactored. Consider wrapping these calls in arrow functions for clarity and safety.
| const handlePushToCloud = async (options = { silent: false }) => { | ||
| if (!supabaseConfig.url || !supabaseConfig.anonKey) { | ||
| if (!options.silent) toast('请先填写 Supabase URL 和 anon key'); | ||
| return; | ||
| } | ||
| const syncId = (supabaseSyncId || '').trim(); | ||
| if (!syncId) { | ||
| if (!options.silent) toast('请设置同步 ID'); | ||
| return; | ||
| } | ||
| setIsSyncing(true); |
There was a problem hiding this comment.
The isSyncing state is not reset to false when the early return conditions are met (lines 456-464). If validation fails, the function returns early and isSyncing remains true, potentially disabling the sync buttons permanently. Move setIsSyncing(true) to after all validation checks.
| const [todos, setTodos] = useState(() => { | ||
| const savedTodos = localStorage.getItem('todos'); | ||
| return savedTodos ? JSON.parse(savedTodos) : [ | ||
| { id: 1, text: 'Hello World', completed: false } | ||
| ]; | ||
| }); |
There was a problem hiding this comment.
The todos state is initialized from localStorage in Index.jsx, but this creates data duplication since TodoWidget also initializes from the same localStorage key. When the component first loads before cloud sync occurs, there could be inconsistencies. Consider removing the initialization from TodoWidget since Index.jsx now manages this state centrally.
| export const getStoredSyncId = () => { | ||
| try { | ||
| const saved = localStorage.getItem(SYNC_ID_STORAGE_KEY); | ||
| return saved || getSupabaseClientId(); | ||
| } catch (error) { | ||
| console.error("Failed to read sync id", error); | ||
| return getSupabaseClientId(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
The getStoredSyncId function returns getSupabaseClientId() as a fallback when saved is falsy. However, this means an empty string stored in localStorage would be replaced with the client ID. This behavior might be unintended - if a user explicitly clears the sync ID (setting it to empty), the function should respect that choice rather than falling back to the client ID.
| -- 示例策略(匿名访问):如果只你自己使用该项目,可以放宽为 anon 全权限 | ||
| create policy "anon read own state" on navinocode_states | ||
| for select using (true); | ||
| create policy "anon upsert state" on navinocode_states | ||
| for insert with check (true); | ||
| create policy "anon update state" on navinocode_states | ||
| for update using (true); |
There was a problem hiding this comment.
The example RLS policies for navinocode_states use using (true) / with check (true) for the anon role, which effectively grant any holder of the anon key full read/write access to all rows in this table. In a typical front-end–only Supabase setup, every end user gets the same anon key and can therefore enumerate and overwrite all other users' synced state via crafted Supabase client calls. To avoid cross-tenant data exposure and tampering, restrict these policies to only allow access to rows bound to the caller (for example by matching client_id to a claim in the JWT or auth.uid()) instead of unconditional true.
| -- 示例策略(匿名访问):如果只你自己使用该项目,可以放宽为 anon 全权限 | |
| create policy "anon read own state" on navinocode_states | |
| for select using (true); | |
| create policy "anon upsert state" on navinocode_states | |
| for insert with check (true); | |
| create policy "anon update state" on navinocode_states | |
| for update using (true); | |
| -- 示例策略(多用户安全访问):仅允许用户访问自己的数据 | |
| create policy "anon read own state" on navinocode_states | |
| for select using (client_id = auth.uid()); | |
| create policy "anon upsert state" on navinocode_states | |
| for insert with check (client_id = auth.uid()); | |
| create policy "anon update state" on navinocode_states | |
| for update using (client_id = auth.uid()); |
验证没问题