Refactor: Treat local store as a cache, never overwriting DB on sync#6
Refactor: Treat local store as a cache, never overwriting DB on sync#6threehymns wants to merge 5 commits intomainfrom
Conversation
I've modified `SyncManager.sync` to prevent the automatic upsert of all local items during the sync process. Now, sync primarily fetches remote changes to update the local cache. New, locally-created items (not yet in the database) are identified as 'localOnlyItems' and are inserted into the database during the sync. Existing items that have local modifications will not have those modifications pushed to the server via the main sync operation; instead, the server's version will refresh the local cache. This change ensures that the database remains the source of truth and the local store acts as a cache, updated by server changes either through full sync or realtime updates.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
""" WalkthroughThe Changes
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/services/SyncManager.ts (3)
30-33: Avoidany; provide a typed projection to satisfy ESLint and future maintainability
remoteRowsandremoteItemsare currently typed asany[], which triggers the ESLint errors reported in the static‐analysis section. Whenever the compiler can’t reason about the data shape, downstream refactors become hazardous.Two lightweight options:
-const remoteRows = remoteRowsData || []; -const remoteItems = (remoteRows as any[]).map(r => this.cfg.mapRow(r)); -const remoteIds = new Set(remoteItems.map((item: any) => (item as any).id)); +type RemoteRow = Record<string, unknown>; // or define a generated type for the table +const remoteRows: RemoteRow[] = (remoteRowsData ?? []) as RemoteRow[]; +const remoteItems = remoteRows.map(row => this.cfg.mapRow(row)); + +type IdOf<T> = T extends { id: infer K } ? K : never; +const remoteIds = new Set<IdOf<typeof remoteItems[number]>>( + remoteItems.map(item => (item as unknown as { id: IdOf<typeof item> }).id) +);Or, if you can generate table-row types from Supabase, import those instead of the hand-rolled
Record<string, unknown>.This keeps the compiler happy and prevents silent type regressions down the road.
🧰 Tools
🪛 ESLint
[error] 31-31: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 32-32: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 32-32: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
34-36: Hard-codedidassumption limits reuse
localOnlyItemsare detected via(item as any).id.
If a consumer supplies items whose primary key is not literallyid, this silently breaks.Consider extending
SyncConfigwith anidSelector: (item: Local) => string | number(defaulting to the current behaviour). That removes the hidden coupling and makes the class fully reusable across tables with composite or differently-named keys.🧰 Tools
🪛 ESLint
[error] 35-35: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
38-52: Optional: useupsertwith minimal columns to avoid race conditionsIf multiple clients can create the same item offline (e.g., deterministic UUIDs), an
insertcall will conflict once the first succeeds.
A conservative alternative is:.from(this.cfg.table) .upsert(payload, { onConflict: "id" })Passing only the columns needed for creation (id, user_id, created_at) minimises the “overwrite-remote” risk while eliminating duplicate-key failures. Evaluate whether this matches your data-model guarantees.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
src/services/SyncManager.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/services/SyncManager.ts (1)
src/services/noteService.ts (1)
retryWithBackoff(15-28)
🪛 ESLint
src/services/SyncManager.ts
[error] 31-31: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 32-32: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 32-32: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 35-35: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
and swallow after the retries are exhausted Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/services/SyncManager.ts (3)
19-19: Improve comment to clarify the numbered stepsThe comment is good for documenting the synchronization flow, but consider expanding it to summarize what all 4 steps do, since you've numbered them. This would give developers a quick understanding of the overall process.
- // 1. Fetch remote items (optionally incremental) - (existing code) + // 1. Fetch remote items (optionally incremental by timestamp) + // 2. Detect local-only items (items not yet on the server) + // 3. Insert local-only items into the remote database + // 4. Return combined list of remote items and local-only items
60-63: Document the behavior change in the method's JSDoc commentThe significant change in synchronization behavior (treating local as cache, not pushing local modifications) should be documented in the method's JSDoc comment to help future developers understand the design decision.
/** * Sync localItems to remote and fetch remote items. + * + * This method treats the local store as a cache. It fetches remote items and + * only sends local-only items (new items) to the server. Local modifications to + * existing items are not pushed to the server during sync - the server remains + * the source of truth. * * @param since ISO timestamp to filter remote updates (inclusive) */
31-32: Address ESLint warnings aboutanytypesThere are several ESLint warnings about using
anytypes in this code. Consider adding more specific types to improve type safety.-const remoteItems = (remoteRows as any[]).map(r => this.cfg.mapRow(r)); -const remoteIds = new Set(remoteItems.map((item: any) => (item as any).id)); +// Define a type for the raw database rows +type RemoteRow = Record<string, unknown> & { id: string }; + +const remoteItems = (remoteRows as RemoteRow[]).map(r => this.cfg.mapRow(r)); +const remoteIds = new Set(remoteItems.map(item => item.id));🧰 Tools
🪛 ESLint
[error] 31-31: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 32-32: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 32-32: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
src/services/SyncManager.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/services/SyncManager.ts (1)
src/services/noteService.ts (1)
retryWithBackoff(15-28)
🪛 ESLint
src/services/SyncManager.ts
[error] 31-31: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 32-32: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 32-32: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 35-35: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🔇 Additional comments (2)
src/services/SyncManager.ts (2)
27-30: Improved error handling and defensive programmingGood improvements in error handling by explicitly throwing fetch errors and providing a fallback to an empty array when data is null. This makes the code more robust.
39-57: Fixed retry mechanism correctly implements previous review feedbackThe implementation now correctly allows
retryWithBackoffto retry failed insertions by:
- Throwing the error inside the callback
- Only catching and logging after all retries are exhausted
This is exactly what was suggested in the previous review and properly addresses the issue.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/services/SyncManager.ts (2)
10-10: Type safety improvement needed formapRow.The
mapRowfunction uses theanytype which bypasses TypeScript's type checking. Consider using a more specific type that represents the structure of the rows returned from Supabase.- mapRow: (row: any) => Local; + mapRow: (row: Record<string, unknown>) => Local;🧰 Tools
🪛 ESLint
[error] 10-10: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
64-67: Document the implications of returning local-only items.The approach of returning both remote items and local-only items (even failed ones) ensures data consistency between sync operations, but may not be immediately clear to other developers.
Consider adding JSDoc comments that explain this pattern and its implications more explicitly:
/** * Sync localItems to remote and fetch remote items. * * @param since ISO timestamp to filter remote updates (inclusive) + * @returns Array of items from the server, plus any local-only items. + * Note: Local modifications to items that exist remotely will NOT be + * propagated to the server. The remote version will take precedence. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
src/services/SyncManager.ts(1 hunks)
🧰 Additional context used
🪛 ESLint
src/services/SyncManager.ts
[error] 10-10: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🔇 Additional comments (8)
src/services/SyncManager.ts (8)
4-6: Good addition of theIdentifiableinterface.Creating this interface ensures type safety by formalizing the requirement for objects to have an
idproperty. This is a solid foundation for the type constraints used throughout the file.
8-8: Good update to the generic constraint.Updating
SyncConfigto requireLocalto extendIdentifiableimproves type safety and makes the interface requirements explicit.
15-15: Good update to the class generic constraint.Using the
Identifiableconstraint on theSyncManagerclass ensures that all operations can safely use theidproperty without type casting.
23-67: Sync logic refactoring aligns with the PR objectives.The refactored sync logic properly treats the local store as a cache by:
- Fetching remote items
- Identifying local-only items
- Inserting only new local items
- Returning a merged list that prioritizes remote data
This implementation ensures the server remains the source of truth while still preserving new local items.
35-36: Improvements in remote items handling.You've improved the code by safely handling the case where
remoteRowsDatamight be null, and by directly using the typedidproperty now thatLocalextendsIdentifiable.
39-39: Type safety improvement inlocalOnlyItemsfiltering.The previous code used a type cast to
anyto access theidproperty. Now with theIdentifiableconstraint, you can safely accessitem.iddirectly, which is more type-safe.
43-61: Proper implementation of error handling with retries.Your implementation correctly addresses the previous review comment about retry behavior:
- Errors are now thrown inside the retry callback, allowing
retryWithBackoffto retry- The error is only caught and logged after all retries are exhausted
- The sync process continues even if inserts fail
This maintains non-blocking behavior while giving transient errors a chance to clear up through retries.
65-66: Good inline documentation of the caching strategy.The comments clearly explain the caching strategy where the local cache is updated with server data plus locally created items. This helps future developers understand the design decision.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/services/SyncManager.ts (5)
83-84: Consider a more type-safe approach for dynamic property access.Using
as anyfor dynamic property access bypasses TypeScript's type checking. Consider creating a type-safe utility for this pattern.- const { [this.cfg.updatedAtColumn || 'updatedAt']: localUpdatedAt, ...localWithoutTs } = localItem as any; - const { [this.cfg.updatedAtColumn || 'updatedAt']: remoteUpdatedAt, ...remoteWithoutTs } = remoteItem as any; + // Type-safe dynamic property access + type ItemWithTimestamp = Local & Record<string, unknown>; + const timestampKey = this.cfg.updatedAtColumn || 'updatedAt'; + + const localUpdatedAt = (localItem as ItemWithTimestamp)[timestampKey]; + const { [timestampKey]: _, ...localWithoutTs } = localItem as ItemWithTimestamp; + + const remoteUpdatedAt = (remoteItem as ItemWithTimestamp)[timestampKey]; + const { [timestampKey]: __, ...remoteWithoutTs } = remoteItem as ItemWithTimestamp;🧰 Tools
🪛 ESLint
[error] 83-83: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 84-84: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
130-165: Robust handling of modified items with proper conflict resolution.The modified items handling implements a "last-write-wins" strategy based on timestamps, with proper error handling and retries.
Consider adding documentation about what happens when the local timestamp is older than the remote one (currently, local changes are discarded in favor of remote data).
// Only update if local changes are actually newer or if there are content changes + // If local changes are older, they will be discarded and the remote version will be used if (isLocalNewer) {🧰 Tools
🪛 ESLint
[error] 145-145: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
145-145: Type-unsafe property assignment.The use of
as anyfor dynamic property assignment bypasses TypeScript's type checking.- (local as any)[this.cfg.updatedAtColumn] = now; + // Type-safe dynamic property assignment + (local as Record<string, unknown>)[this.cfg.updatedAtColumn] = now;🧰 Tools
🪛 ESLint
[error] 145-145: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
11-11: Consider using a more specific type for mapRow parameter.Using
anyfor the row type reduces type safety. Consider using a generic type parameter or a more specific type that represents the database row structure.- mapRow: (row: any) => Local; + mapRow: (row: Record<string, unknown>) => Local;🧰 Tools
🪛 ESLint
[error] 11-11: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
32-32: Consider using a more specific type for remote rows.Using
any[]forallRemoteRowsreduces type safety. Consider using a more specific type that represents the structure of the database rows.- let allRemoteRows: any[] = []; + let allRemoteRows: Record<string, unknown>[] = [];🧰 Tools
🪛 ESLint
[error] 32-32: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
src/lib/utils.ts(1 hunks)src/services/SyncManager.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/services/SyncManager.ts (2)
src/lib/utils.ts (1)
deepEqual(48-83)src/services/noteService.ts (1)
retryWithBackoff(15-28)
🪛 ESLint
src/services/SyncManager.ts
[error] 11-11: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 32-32: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 83-83: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 84-84: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 145-145: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🔇 Additional comments (6)
src/lib/utils.ts (1)
48-83: Well-implemented deep equality function.The
deepEqualfunction provides robust deep comparison with comprehensive handling of various data types, cyclic references, built-in objects, and nested structures. This will improve the accuracy of object comparisons compared to simple serialization approaches.src/services/SyncManager.ts (5)
5-7: Good addition of the Identifiable interface.Creating a reusable interface that enforces the presence of an
idproperty improves type safety throughout the codebase.
9-19: Well-documented pagination support.The addition of the optional
pageSizeparameter with clear documentation improves the API's flexibility and helps users understand the performance implications.🧰 Tools
🪛 ESLint
[error] 11-11: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
29-64: Effective implementation of incremental pagination.The implementation correctly handles cursor-based pagination, ensuring all remote items are fetched efficiently without unnecessary data transfer. The approach of fetching pages until no more data remains is robust.
🧰 Tools
🪛 ESLint
[error] 32-32: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
111-128: Proper error handling with retry logic.The implementation correctly throws errors inside the retry callback to trigger retries, and then catches them after all retries are exhausted. This addresses a previous issue where errors were swallowed inside the callback, preventing retry attempts.
167-183: Comprehensive merging strategy for synchronized items.The merging logic effectively combines remote-only, local-only, modified, and unchanged items while avoiding duplicates. The use of a Set for tracking processed IDs is efficient.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/services/SyncManager.ts (3)
29-64: Well-implemented incremental paginationThe pagination logic is well-structured and robust, handling edge cases properly:
- Uses configurable page size
- Implements cursor-based pagination
- Handles empty results
- Detects end of data
- Properly accumulates results
Consider adding a type for the remote rows instead of using
any[]:-let allRemoteRows: any[] = []; +type RemoteRow = Record<string, unknown>; +let allRemoteRows: RemoteRow[] = [];🧰 Tools
🪛 ESLint
[error] 32-32: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
83-84: Consider improving type safety in destructuringThe
as anycasts could be avoided by using a more specific type or interface.- const { [this.cfg.updatedAtColumn || 'updatedAt']: localUpdatedAt, ...localWithoutTs } = localItem as any; - const { [this.cfg.updatedAtColumn || 'updatedAt']: remoteUpdatedAt, ...remoteWithoutTs } = remoteItem as any; + type ItemWithTimestamp = Local & Record<string, unknown>; + const updatedAtKey = this.cfg.updatedAtColumn || 'updatedAt'; + const { [updatedAtKey]: localUpdatedAt, ...localWithoutTs } = localItem as ItemWithTimestamp; + const { [updatedAtKey]: remoteUpdatedAt, ...remoteWithoutTs } = remoteItem as ItemWithTimestamp;🧰 Tools
🪛 ESLint
[error] 83-83: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 84-84: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
130-165: Well-implemented conflict resolution for modified itemsThe implementation follows the proper last-write-wins strategy:
- Only updates if local changes are newer
- Sets the updated timestamp on both the payload and local object
- Uses proper error handling with retry
Consider adding a type for the updates array:
- const updates = []; + const updates: Record<string, unknown>[] = [];🧰 Tools
🪛 ESLint
[error] 145-145: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
src/services/SyncManager.ts(1 hunks)
🧰 Additional context used
🪛 ESLint
src/services/SyncManager.ts
[error] 11-11: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 32-32: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 83-83: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 84-84: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 145-145: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🔇 Additional comments (3)
src/services/SyncManager.ts (3)
3-3: Good addition of the deepEqual importUsing a dedicated deep equality function is much better than the previous approach, especially for complex nested objects.
111-128: Excellent error handling with retryWithBackoffThis properly fixes the issue identified in the previous review by:
- Throwing the error inside the callback to trigger retry
- Catching only after all retries are exhausted
- Logging a warning instead of failing the sync
167-183: Clean and comprehensive result mergingThe merging logic properly:
- Combines remote-only and local-only items
- Selects the newer version of modified items
- Includes unchanged items while avoiding duplicates
This ensures a complete and accurate sync result.
| const localTimeRaw = localUpdatedAt ? new Date(localUpdatedAt).getTime() : Number.NEGATIVE_INFINITY; | ||
| const localTime = Number.isNaN(localTimeRaw) ? Number.NEGATIVE_INFINITY : localTimeRaw; | ||
| const remoteTimeRaw = remoteUpdatedAt ? new Date(remoteUpdatedAt).getTime() : Number.NEGATIVE_INFINITY; | ||
| const remoteTime = Number.isFinite(remoteTimeRaw) ? remoteTimeRaw : Number.NEGATIVE_INFINITY; |
There was a problem hiding this comment.
Bug: Incorrect ternary expression for remoteTime calculation
The current expression always evaluates to a boolean instead of the timestamp value.
- const remoteTime = Number.isFinite(remoteTimeRaw) ? Number.isFinite(remoteTimeRaw) : Number.NEGATIVE_INFINITY;
+ const remoteTime = Number.isFinite(remoteTimeRaw) ? remoteTimeRaw : Number.NEGATIVE_INFINITY;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const remoteTime = Number.isFinite(remoteTimeRaw) ? remoteTimeRaw : Number.NEGATIVE_INFINITY; | |
| // before: returns a boolean instead of the timestamp | |
| - const remoteTime = Number.isFinite(remoteTimeRaw) ? Number.isFinite(remoteTimeRaw) : Number.NEGATIVE_INFINITY; | |
| + const remoteTime = Number.isFinite(remoteTimeRaw) ? remoteTimeRaw : Number.NEGATIVE_INFINITY; |
🤖 Prompt for AI Agents
In src/services/SyncManager.ts at line 92, the ternary expression for remoteTime
is incorrect because it uses Number.isFinite(remoteTimeRaw) directly, which
returns a boolean, causing remoteTime to be set to true or false instead of the
intended numeric timestamp. Fix this by checking if remoteTimeRaw is a finite
number and if so, assign remoteTimeRaw itself; otherwise, assign
Number.NEGATIVE_INFINITY.
| export interface Identifiable { | ||
| id: string; | ||
| } | ||
|
|
||
| export interface SyncConfig<Local extends Identifiable> { | ||
| table: string; | ||
| mapRow: (row: any) => Local; | ||
| mapLocal: (item: Local, userId: string) => Record<string, unknown>; | ||
| updatedAtColumn?: string; | ||
| /** | ||
| * Page size for fetching records. Defaults to 1000. | ||
| * Adjust based on your record size and performance needs. | ||
| */ | ||
| pageSize?: number; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Excellent type system improvements
Creating the Identifiable interface and constraining the generic type parameter makes the code more type-safe and self-documenting. The addition of the pageSize parameter with documentation is also a good enhancement.
There's still an any type in the mapRow function that could be improved:
🏁 Script executed:
#!/bin/bash
# Check how this function is used in practice
rg "mapRow" --type ts -A 5 -B 5 src/Length of output: 3941
Use a strongly-typed row parameter in mapRow
While the new Identifiable constraint and pageSize docs are great, the mapRow: (row: any) => Local still relies on any, which erodes type safety. Please introduce a generic for the remote row type and replace any with a well-defined type.
• In src/services/SyncManager.ts, update the interface:
-export interface SyncConfig<Local extends Identifiable> {
- table: string;
- mapRow: (row: any) => Local;
+export interface SyncConfig<
+ Local extends Identifiable,
+ Remote = Record<string, unknown>
+> {
+ table: string;
+ mapRow: (row: Remote) => Local;
mapLocal: (item: Local, userId: string) => Record<string, unknown>;
updatedAtColumn?: string;
pageSize?: number;
}• Adjust existing usages:
src/store/noteStore.ts— replace(row: any)with(row: NoteRow)orRecord<string, unknown>.src/services/noteService.ts— similarly type therowparameter.
This change avoids any and makes each mapRow implementation explicitly typed.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export interface Identifiable { | |
| id: string; | |
| } | |
| export interface SyncConfig<Local extends Identifiable> { | |
| table: string; | |
| mapRow: (row: any) => Local; | |
| mapLocal: (item: Local, userId: string) => Record<string, unknown>; | |
| updatedAtColumn?: string; | |
| /** | |
| * Page size for fetching records. Defaults to 1000. | |
| * Adjust based on your record size and performance needs. | |
| */ | |
| pageSize?: number; | |
| } | |
| export interface Identifiable { | |
| id: string; | |
| } | |
| export interface SyncConfig< | |
| Local extends Identifiable, | |
| Remote = Record<string, unknown> | |
| > { | |
| table: string; | |
| mapRow: (row: Remote) => Local; | |
| mapLocal: (item: Local, userId: string) => Record<string, unknown>; | |
| updatedAtColumn?: string; | |
| /** | |
| * Page size for fetching records. Defaults to 1000. | |
| * Adjust based on your record size and performance needs. | |
| */ | |
| pageSize?: number; | |
| } |
🧰 Tools
🪛 ESLint
[error] 11-11: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🤖 Prompt for AI Agents
In src/services/SyncManager.ts lines 5 to 19, the mapRow function parameter is
typed as any, reducing type safety. Introduce a new generic type parameter for
the remote row type in the SyncConfig interface and replace the any type in
mapRow with this generic type. Then update all existing usages of SyncConfig,
such as in src/store/noteStore.ts and src/services/noteService.ts, to specify
the appropriate row type instead of any, ensuring each mapRow implementation
uses a strongly typed parameter.
1 participant
I've modified
SyncManager.syncto prevent the automatic upsert of all local items during the sync process. Now, sync primarily fetches remote changes to update the local cache.New, locally-created items (not yet in the database) are identified as 'localOnlyItems' and are inserted into the database during the sync. Existing items that have local modifications will not have those modifications pushed to the server via the main sync operation; instead, the server's version will refresh the local cache.
This change ensures that the database remains the source of truth and the local store acts as a cache, updated by server changes either through full sync or realtime updates.
Summary by CodeRabbit