refactor: cut all database call sites over to the facade#7407
Open
diegolmello wants to merge 3 commits into
Open
refactor: cut all database call sites over to the facade#7407diegolmello wants to merge 3 commits into
diegolmello wants to merge 3 commits into
Conversation
Introduce a temporary compatibility layer that exposes the WatermelonDB
public API (Database, Collection, Query, Model, decorators, Q clauses,
sanitizedRaw, appSchema/tableSchema) on top of the synchronous Drizzle
expo-sqlite driver. A fetched Drizzle row is column-keyed and therefore
structurally identical to WatermelonDB's _raw, so the facade Model wraps
the row directly and field getters read _raw[column].
This lets the ~80 existing @nozbe/watermelondb import sites be re-pointed
onto the facade without touching call-site logic, ahead of removing the
WatermelonDB package entirely in a later step.
Notable deviations from WatermelonDB, required by Drizzle:
- sanitizedRaw emits { id, ...coercedColumns } only — no _status/_changed,
which have no Drizzle columns; a random 16-char id is generated when absent.
- write/batch wrap the synchronous Drizzle transaction in a Promise surface;
a single-writer WriterQueue serializes writers.
- observe()/observeWithColumns() bridge expo-sqlite's change listener to RxJS,
filtering by database file + table, debouncing 16ms, and structurally
sharing unchanged row references.
Verified: tsc, eslint, and 32 unit tests pass on a fresh empty database.
Comparison operators (eq, notEq, gt, gte, lt, lte, like, notLike, oneOf) take
only the right-hand value and return a Comparison; Q.where(column, valueOrComparison)
wraps it, treating a raw value as an implicit eq. This matches every call site
(e.g. Q.where('ts', Q.gt(date)), Q.where('id', Q.oneOf(ids))) ahead of the cutover.
Null comparisons now lower to IS NULL / IS NOT NULL rather than = NULL, which is
never true in SQL — required for the many Q.where(col, null) and Q.notEq(null) sites.
Replace @nozbe/watermelondb imports across the app with the WatermelonDB-shaped facade over the Drizzle/expo-sqlite driver, so the app runs on the encrypted SQLCipher database while keeping call sites unchanged. Add the table/model maps the facade needs to construct each Database, wire the servers and app schemas through it, and add an ESLint rule banning new direct watermelondb imports. WatermelonDB stays installed only for the legacy migration reader; no JS runtime path uses it anymore.
10 tasks
Contributor
WalkthroughThis PR replaces WatermelonDB with a custom facade layer backed by Drizzle ORM and ChangesDatabase Facade Implementation and Migration
Sequence Diagram(s)sequenceDiagram
participant App
participant connect
participant DB
participant openServerDb
participant Database
participant WriterQueue
participant DrizzleTransaction
App->>connect: connect({ server })
connect->>DB: await setActiveDB(server)
DB->>openServerDb: openServerDb(server)
openServerDb-->>DB: DbHandle (with migrations run)
DB->>Database: new Database(handle, appSchema, appTableMap, appModelMap)
Database-->>DB: TAppDatabase
DB-->>connect: activeDB set
connect->>App: Promise resolves
App->>Database: write(fn)
Database->>WriterQueue: enqueue(fn)
WriterQueue->>fn: execute (serialized)
fn->>Database: batch(models)
Database->>DrizzleTransaction: INSERT/UPDATE/DELETE per _pendingOp
DrizzleTransaction-->>Database: committed
Database-->>App: Promise<void>
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (3)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/lib/database/driver/connection.ts (2)
176-201:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClose the SQLite handle when open/migration fails.
If
applyOpenPragmas()(Line 186) ormigrate()(Line 196) throws,sqliteremains open and uncached, leaking a live handle and potential file lock.💡 Proposed fix
async function openDb<K extends DbKind>(dbName: string, kind: K): Promise<DbHandle<K>> { const cached = _registry.get(dbName); if (cached) { return cached as DbHandle<K>; } const keyHex = await getOrCreateDatabaseKey(dbName); - - const sqlite = await openDatabaseAsync(dbName, { enableChangeListener: true }, DB_DIRECTORY); - - await applyOpenPragmas(sqlite, keyHex); - - const schema = kind === 'servers' ? serversSchema : appSchema; - // The conditional type SchemaForKind<K> cannot be narrowed by the JS runtime check above; - // casting through unknown is the standard TS pattern for this shape. - const db = drizzle(sqlite, { schema }) as unknown as ExpoSQLiteDatabase<SchemaForKind<K>>; - - // Apply the schema DDL on the freshly keyed handle. The migrator creates tables on first - // open and tracks applied migrations in __drizzle_migrations, so re-opens are no-ops. - const migrations = (kind === 'servers' ? serversMigrations : appMigrations) as MigrationConfig; - await migrate(db, migrations); - - const handle: DbHandle<K> = { db, sqlite, dbName }; - _registry.set(dbName, handle as DbHandle); - return handle; + let sqlite: SQLiteDatabase | undefined; + try { + sqlite = await openDatabaseAsync(dbName, { enableChangeListener: true }, DB_DIRECTORY); + await applyOpenPragmas(sqlite, keyHex); + + const schema = kind === 'servers' ? serversSchema : appSchema; + const db = drizzle(sqlite, { schema }) as unknown as ExpoSQLiteDatabase<SchemaForKind<K>>; + + const migrations = (kind === 'servers' ? serversMigrations : appMigrations) as MigrationConfig; + await migrate(db, migrations); + + const handle: DbHandle<K> = { db, sqlite, dbName }; + _registry.set(dbName, handle as DbHandle); + return handle; + } catch (e) { + if (sqlite) { + try { + await sqlite.closeAsync(); + } catch { + // ignore cleanup failure; preserve original error + } + } + throw e; + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/lib/database/driver/connection.ts` around lines 176 - 201, The issue in the openDb function is that if either applyOpenPragmas or migrate throws an error, the sqlite handle opened by openDatabaseAsync remains unclosed and uncached, causing a resource leak. Wrap the applyOpenPragmas and migrate operations in a try-catch block, and in the catch block, close the sqlite handle by calling its appropriate close method before re-throwing the error to ensure proper cleanup.
71-85:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
deriveServerDbName()can collide with the global servers DB filename.Line 71 reserves
default.dbforopenServersDb(), and Line 84 can generate that same value for app DBs (e.g., server URLhttps://default). That can route app and servers traffic to the same file/registry key.Also applies to: 210-220
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/lib/database/driver/connection.ts` around lines 71 - 85, The deriveServerDbName() function can generate the same filename as the reserved DEFAULT_DB_NAME constant ('default.db'), which occurs when processing server URLs like 'https://default'. This creates a collision where app database traffic and global servers database traffic get routed to the same file. Fix this by adding a collision detection and prevention mechanism in deriveServerDbName(): after deriving the sanitized database name, check if it matches DEFAULT_DB_NAME and if so, apply a differentiating suffix or alternative naming strategy to ensure uniqueness. Apply the same fix to the other similar logic mentioned at lines 210-220 to maintain consistency across all database filename derivation.
🧹 Nitpick comments (1)
app/lib/database/facade/decorators.ts (1)
119-125: ⚡ Quick winUse
_jsonDecoratorCachein@jsongetter to preserve memoized behavior.Line 119-125 reparses and re-sanitizes on every access, while
Modelalready allocates_jsonDecoratorCachefor this decorator path. This adds avoidable work and can break stable-reference expectations.Suggested patch
get(this: AnyModel): unknown { const model = this.asModel; const rawValue = model._getRaw(rawFieldName); + const cached = model._jsonDecoratorCache[rawFieldName]; + if (cached && cached[0] === rawValue) { + return cached[1]; + } const parsedValue = parseJSON(rawValue); const sanitized = sanitizer(parsedValue, model); + model._jsonDecoratorCache[rawFieldName] = [rawValue, sanitized]; return sanitized; },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/lib/database/facade/decorators.ts` around lines 119 - 125, The `get` method in the `@json` decorator is re-parsing and re-sanitizing the value on every access instead of using the memoization cache. Modify the getter to first check if a cached value exists in the model's `_jsonDecoratorCache` using the `rawFieldName` as the key. If cached, return it immediately. Otherwise, perform the existing parsing and sanitization logic (parseJSON and sanitizer calls), store the result in `_jsonDecoratorCache` with the `rawFieldName` key, and then return the sanitized value. This preserves the memoized behavior and avoids redundant parsing and sanitization work on subsequent accesses.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/lib/database/facade/Collection.ts`:
- Around line 61-66: The truthy check on filter?.id at line 61 incorrectly skips
the id filter when filter.id is a valid falsy value like 0, false, or an empty
string, causing the code to fall back to full-table selection. Replace the
truthy check if (filter?.id) with a presence check that distinguishes between
falsy values and undefined/null, such as checking if ('id' in filter) or using
filter?.id !== undefined, so that the where clause with eq(idCol, filter.id) is
applied regardless of whether the id value is falsy.
In `@app/lib/database/facade/Database.ts`:
- Around line 79-110: The issue is that model._pendingOp is being cleared inside
the transaction loop at line 109, which means if a subsequent database operation
fails and the transaction rolls back, the earlier models will already have
_pendingOp set to null, causing the retry logic to silently skip them. Move the
model._pendingOp = null assignment outside of the transaction callback function
so that it only executes after the entire transaction successfully completes,
ensuring that if any operation fails and the transaction rolls back, all models
still have their _pendingOp set and can be retried.
In `@app/lib/database/facade/decorators.ts`:
- Around line 242-249: The `observeRow` function used in the return statement
suppresses null values, but the declared return type `Observable<T | null>`
indicates the observable should emit null when relations are removed or deleted.
Replace the use of `observeRow` with an alternative observation mechanism that
preserves null emissions, ensuring that when the foreign key is cleared or the
related row is deleted, the observer receives a null value as expected by the
contract defined in the return type signature.
- Around line 173-183: The Collection constructor in the `@children` decorator is
being passed the generic Model class instead of the child table's specific model
class, which causes rows to lose child-specific decorators and getters. Extract
the actual model class from the childCollection (similar to how the table,
schema, _drizzleTable, and _handle are being extracted via type assertions) and
pass that model class as the final argument to the Collection constructor
instead of the generic Model parameter.
In `@app/lib/database/facade/observe.ts`:
- Around line 73-82: Wrap the fetchFn() calls in try-catch blocks throughout the
file to properly handle exceptions through RxJS error handling. In the emit
function (starting at line 73), wrap the const fresh = fetchFn() call in a
try-catch block, and in the catch clause, route the error to subscriber.error()
before returning. Apply the same pattern to the other two emit paths mentioned
(around lines 108-112 and 143-157) to ensure all fetchFn() exceptions are
properly caught and passed through subscriber.error() instead of propagating as
unhandled runtime exceptions when triggered by setTimeout callbacks.
- Around line 177-184: The sameByColumns function only compares watched column
values at matching array indices without checking row identity. If rows reorder
or a different row with identical watched column values replaces a row at the
same index, the function incorrectly returns true and suppresses necessary
update emissions. Add a row identity check before comparing column values in
sameByColumns to ensure that even if two rows have matching watched column
values, they are still treated as different if they have different ids. Compare
the row identities first (using the id property from HasId) to catch reordered
or replaced rows.
In `@app/lib/database/facade/writer.ts`:
- Around line 19-24: The enqueue method fails to handle synchronous exceptions
thrown by fn(). When fn() throws synchronously before returning a promise, the
second .then() handler with dual undefined callbacks catches the error and
resolves to undefined instead of rejecting the caller's promise, causing resolve
and reject to never be invoked. Wrap the fn() call in a try-catch block within
the first .then() callback to catch synchronous exceptions and call
reject(error) when they occur, ensuring the caller's promise always settles
regardless of whether fn() throws synchronously or asynchronously.
In `@app/lib/database/index.ts`:
- Around line 60-63: The `setActiveDB()` method has a race condition where
multiple rapid calls can result in out-of-order async completion, causing stale
database state to overwrite newer state. Add a request identifier (such as a
counter) that increments each time `setActiveDB()` is invoked, capture this
identifier in the async operation, and only update `this.databases.activeDB`
when the returned handle belongs to the most recent request. This ensures that
slower earlier requests do not overwrite more recent database switches.
In `@ios/Shared/RocketChat/Database.swift`:
- Around line 73-79: The sqlite3_open call assumes the SQLite/ subdirectory
already exists, but on fresh installs or upgrades this directory may not be
present, causing the operation to fail. Before calling sqlite3_open with the
path variable, create the sqliteDir directory using
FileManager.default.createDirectory with withIntermediateDirectories set to
true. Add appropriate error handling to ensure the directory creation succeeds
before attempting to open the database connection.
---
Outside diff comments:
In `@app/lib/database/driver/connection.ts`:
- Around line 176-201: The issue in the openDb function is that if either
applyOpenPragmas or migrate throws an error, the sqlite handle opened by
openDatabaseAsync remains unclosed and uncached, causing a resource leak. Wrap
the applyOpenPragmas and migrate operations in a try-catch block, and in the
catch block, close the sqlite handle by calling its appropriate close method
before re-throwing the error to ensure proper cleanup.
- Around line 71-85: The deriveServerDbName() function can generate the same
filename as the reserved DEFAULT_DB_NAME constant ('default.db'), which occurs
when processing server URLs like 'https://default'. This creates a collision
where app database traffic and global servers database traffic get routed to the
same file. Fix this by adding a collision detection and prevention mechanism in
deriveServerDbName(): after deriving the sanitized database name, check if it
matches DEFAULT_DB_NAME and if so, apply a differentiating suffix or alternative
naming strategy to ensure uniqueness. Apply the same fix to the other similar
logic mentioned at lines 210-220 to maintain consistency across all database
filename derivation.
---
Nitpick comments:
In `@app/lib/database/facade/decorators.ts`:
- Around line 119-125: The `get` method in the `@json` decorator is re-parsing
and re-sanitizing the value on every access instead of using the memoization
cache. Modify the getter to first check if a cached value exists in the model's
`_jsonDecoratorCache` using the `rawFieldName` as the key. If cached, return it
immediately. Otherwise, perform the existing parsing and sanitization logic
(parseJSON and sanitizer calls), store the result in `_jsonDecoratorCache` with
the `rawFieldName` key, and then return the sanitized value. This preserves the
memoized behavior and avoids redundant parsing and sanitization work on
subsequent accesses.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 81a6d09c-6e06-4b53-bf0e-5f2bd5d07766
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (109)
.eslintrc.jsapp/containers/Avatar/useAvatarETag.tsapp/containers/MessageComposer/MessageComposer.tsxapp/containers/MessageComposer/components/SendThreadToChannel.tsxapp/containers/MessageComposer/hooks/useAutocomplete.tsapp/containers/MessageErrorActions.tsxapp/definitions/IEmoji.tsapp/definitions/ILoggedUser.tsapp/definitions/IMessage.tsapp/definitions/IPermission.tsapp/definitions/IRole.tsapp/definitions/IRoom.tsapp/definitions/IServer.tsapp/definitions/IServerHistory.tsapp/definitions/ISettings.tsapp/definitions/ISlashCommand.tsapp/definitions/ISubscription.tsapp/definitions/IThread.tsapp/definitions/IThreadMessage.tsapp/definitions/IUpload.tsapp/definitions/IUser.tsapp/lib/database/driver/__tests__/connection.test.tsapp/lib/database/driver/connection.tsapp/lib/database/facade/Collection.tsapp/lib/database/facade/Database.tsapp/lib/database/facade/Model.tsapp/lib/database/facade/Q.tsapp/lib/database/facade/Query.tsapp/lib/database/facade/__tests__/facade.test.tsapp/lib/database/facade/decorators.tsapp/lib/database/facade/index.tsapp/lib/database/facade/observe.tsapp/lib/database/facade/schema.tsapp/lib/database/facade/translate.tsapp/lib/database/facade/writer.tsapp/lib/database/index.tsapp/lib/database/interfaces.tsapp/lib/database/model/CustomEmoji.jsapp/lib/database/model/FrequentlyUsedEmoji.jsapp/lib/database/model/Message.jsapp/lib/database/model/Permission.jsapp/lib/database/model/Role.jsapp/lib/database/model/Room.jsapp/lib/database/model/ServersHistory.jsapp/lib/database/model/Setting.jsapp/lib/database/model/SlashCommand.jsapp/lib/database/model/Subscription.jsapp/lib/database/model/Thread.jsapp/lib/database/model/ThreadMessage.jsapp/lib/database/model/Upload.jsapp/lib/database/model/User.jsapp/lib/database/model/servers/Server.jsapp/lib/database/model/servers/User.jsapp/lib/database/schema/app.jsapp/lib/database/schema/servers.jsapp/lib/database/services/Subscription.tsapp/lib/database/tableMaps.tsapp/lib/encryption/encryption.tsapp/lib/hooks/useFrequentlyUsedEmoji.tsapp/lib/methods/AudioManager.tsapp/lib/methods/emojis.tsapp/lib/methods/getCustomEmojis.tsapp/lib/methods/getPermissions.tsapp/lib/methods/getRoles.tsapp/lib/methods/getSettings.tsapp/lib/methods/getSlashCommands.tsapp/lib/methods/getThreadName.tsapp/lib/methods/getUsersPresence.tsapp/lib/methods/handleMediaDownload.tsapp/lib/methods/helpers/findSubscriptionsRooms.tsapp/lib/methods/helpers/markMessagesRead.tsapp/lib/methods/loadThreadMessages.tsapp/lib/methods/logout.tsapp/lib/methods/search.tsapp/lib/methods/sendFileMessage/sendFileMessage.tsapp/lib/methods/sendFileMessage/utils.tsapp/lib/methods/sendMessage.tsapp/lib/methods/subscriptions/room.tsapp/lib/methods/subscriptions/rooms.tsapp/lib/methods/updateMessages.tsapp/lib/services/connect.tsapp/sagas/createChannel.jsapp/sagas/createDiscussion.jsapp/sagas/login.jsapp/sagas/messages.jsapp/sagas/rooms.jsapp/sagas/selectServer.tsapp/views/AddExistingChannelView/index.tsxapp/views/NewMessageView/index.tsxapp/views/NewServerView/hooks/useServersHistory.tsxapp/views/RoomActionsView/index.tsxapp/views/RoomInfoEditView/hooks/useRoomDeletionActions.tsxapp/views/RoomMembersView/helpers.tsapp/views/RoomView/List/hooks/buildVisibleSystemTypesClause.tsapp/views/RoomView/List/hooks/useMessages.tsapp/views/RoomView/UploadProgress.tsxapp/views/RoomView/index.tsxapp/views/RoomsListView/hooks/useSubscriptions.tsapp/views/SearchMessagesView/index.tsxapp/views/SelectServerView.tsxapp/views/SelectedUsersView/index.tsxapp/views/ShareListView/index.tsxapp/views/ShareView/index.tsxapp/views/TeamChannelsView.tsxapp/views/ThreadMessagesView/index.tsxbabel.config.jsios/Shared/RocketChat/Database.swiftmetro.config.jspackage.json
📜 Review details
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/definitions/ISettings.tsapp/definitions/IThread.tsapp/definitions/IPermission.tsapp/lib/database/model/CustomEmoji.jsapp/definitions/IUpload.tsapp/lib/database/model/Room.jsapp/definitions/IEmoji.tsapp/lib/database/interfaces.tsapp/lib/database/model/FrequentlyUsedEmoji.jsapp/lib/database/model/Role.jsapp/lib/database/model/servers/User.jsapp/lib/database/model/SlashCommand.jsapp/lib/database/model/Setting.jsapp/lib/database/model/Permission.jsapp/definitions/IThreadMessage.tsapp/lib/database/services/Subscription.tsapp/lib/methods/helpers/markMessagesRead.tsapp/lib/database/facade/writer.tsapp/lib/methods/getSlashCommands.tsapp/views/RoomView/List/hooks/buildVisibleSystemTypesClause.tsapp/lib/methods/getThreadName.tsapp/containers/MessageComposer/components/SendThreadToChannel.tsxapp/views/SelectServerView.tsxapp/definitions/IServerHistory.tsapp/lib/database/facade/schema.tsapp/sagas/createDiscussion.jsapp/lib/database/model/Subscription.jsapp/lib/database/schema/app.jsapp/lib/methods/getRoles.tsapp/definitions/IMessage.tsapp/lib/methods/emojis.tsapp/containers/Avatar/useAvatarETag.tsapp/lib/methods/getSettings.tsapp/sagas/messages.jsapp/views/AddExistingChannelView/index.tsxmetro.config.jsapp/containers/MessageComposer/hooks/useAutocomplete.tsapp/lib/database/model/ServersHistory.jsapp/definitions/IRoom.tsapp/definitions/ILoggedUser.tsapp/views/RoomView/List/hooks/useMessages.tsapp/definitions/IRole.tsapp/definitions/ISubscription.tsapp/lib/database/model/servers/Server.jsapp/lib/methods/sendMessage.tsapp/lib/methods/AudioManager.tsapp/views/NewMessageView/index.tsxapp/containers/MessageErrorActions.tsxbabel.config.jsapp/definitions/IServer.tsapp/lib/database/model/ThreadMessage.jsapp/views/TeamChannelsView.tsxapp/lib/database/facade/index.tsapp/lib/database/model/User.jsapp/lib/hooks/useFrequentlyUsedEmoji.tsapp/definitions/IUser.tsapp/lib/methods/helpers/findSubscriptionsRooms.tsapp/lib/database/model/Upload.jsapp/sagas/rooms.jsapp/sagas/selectServer.tsapp/lib/methods/getUsersPresence.tsapp/views/RoomsListView/hooks/useSubscriptions.tsapp/lib/database/driver/__tests__/connection.test.tsapp/views/SearchMessagesView/index.tsxapp/lib/methods/updateMessages.tsapp/lib/methods/getCustomEmojis.tsapp/views/RoomInfoEditView/hooks/useRoomDeletionActions.tsxapp/lib/methods/search.tsapp/lib/database/facade/Query.tsapp/definitions/ISlashCommand.tsapp/lib/database/facade/translate.tsapp/views/ThreadMessagesView/index.tsxapp/lib/database/model/Thread.jsapp/lib/methods/subscriptions/rooms.tsapp/lib/methods/sendFileMessage/sendFileMessage.tsapp/lib/methods/getPermissions.tsapp/lib/database/tableMaps.tsapp/lib/methods/handleMediaDownload.tsapp/lib/methods/subscriptions/room.tsapp/sagas/createChannel.jsapp/lib/methods/loadThreadMessages.tsapp/lib/methods/sendFileMessage/utils.tsapp/views/RoomMembersView/helpers.tsapp/lib/database/model/Message.jsapp/sagas/login.jsapp/lib/methods/logout.tsapp/views/ShareView/index.tsxapp/lib/database/facade/Q.tsapp/lib/database/facade/Database.tsapp/lib/encryption/encryption.tsapp/views/SelectedUsersView/index.tsxapp/views/ShareListView/index.tsxapp/lib/database/index.tsapp/lib/database/facade/decorators.tsapp/lib/services/connect.tsapp/lib/database/facade/observe.tsapp/views/RoomActionsView/index.tsxapp/lib/database/facade/Model.tsapp/lib/database/facade/Collection.tsapp/lib/database/driver/connection.tsapp/lib/database/facade/__tests__/facade.test.tsapp/views/RoomView/index.tsxapp/views/NewServerView/hooks/useServersHistory.tsxapp/lib/database/schema/servers.jsapp/views/RoomView/UploadProgress.tsxapp/containers/MessageComposer/MessageComposer.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbersUse TypeScript with strict mode enabled
Files:
app/definitions/ISettings.tsapp/definitions/IThread.tsapp/definitions/IPermission.tsapp/definitions/IUpload.tsapp/definitions/IEmoji.tsapp/lib/database/interfaces.tsapp/definitions/IThreadMessage.tsapp/lib/database/services/Subscription.tsapp/lib/methods/helpers/markMessagesRead.tsapp/lib/database/facade/writer.tsapp/lib/methods/getSlashCommands.tsapp/views/RoomView/List/hooks/buildVisibleSystemTypesClause.tsapp/lib/methods/getThreadName.tsapp/containers/MessageComposer/components/SendThreadToChannel.tsxapp/views/SelectServerView.tsxapp/definitions/IServerHistory.tsapp/lib/database/facade/schema.tsapp/lib/methods/getRoles.tsapp/definitions/IMessage.tsapp/lib/methods/emojis.tsapp/containers/Avatar/useAvatarETag.tsapp/lib/methods/getSettings.tsapp/views/AddExistingChannelView/index.tsxapp/containers/MessageComposer/hooks/useAutocomplete.tsapp/definitions/IRoom.tsapp/definitions/ILoggedUser.tsapp/views/RoomView/List/hooks/useMessages.tsapp/definitions/IRole.tsapp/definitions/ISubscription.tsapp/lib/methods/sendMessage.tsapp/lib/methods/AudioManager.tsapp/views/NewMessageView/index.tsxapp/containers/MessageErrorActions.tsxapp/definitions/IServer.tsapp/views/TeamChannelsView.tsxapp/lib/database/facade/index.tsapp/lib/hooks/useFrequentlyUsedEmoji.tsapp/definitions/IUser.tsapp/lib/methods/helpers/findSubscriptionsRooms.tsapp/sagas/selectServer.tsapp/lib/methods/getUsersPresence.tsapp/views/RoomsListView/hooks/useSubscriptions.tsapp/lib/database/driver/__tests__/connection.test.tsapp/views/SearchMessagesView/index.tsxapp/lib/methods/updateMessages.tsapp/lib/methods/getCustomEmojis.tsapp/views/RoomInfoEditView/hooks/useRoomDeletionActions.tsxapp/lib/methods/search.tsapp/lib/database/facade/Query.tsapp/definitions/ISlashCommand.tsapp/lib/database/facade/translate.tsapp/views/ThreadMessagesView/index.tsxapp/lib/methods/subscriptions/rooms.tsapp/lib/methods/sendFileMessage/sendFileMessage.tsapp/lib/methods/getPermissions.tsapp/lib/database/tableMaps.tsapp/lib/methods/handleMediaDownload.tsapp/lib/methods/subscriptions/room.tsapp/lib/methods/loadThreadMessages.tsapp/lib/methods/sendFileMessage/utils.tsapp/views/RoomMembersView/helpers.tsapp/lib/methods/logout.tsapp/views/ShareView/index.tsxapp/lib/database/facade/Q.tsapp/lib/database/facade/Database.tsapp/lib/encryption/encryption.tsapp/views/SelectedUsersView/index.tsxapp/views/ShareListView/index.tsxapp/lib/database/index.tsapp/lib/database/facade/decorators.tsapp/lib/services/connect.tsapp/lib/database/facade/observe.tsapp/views/RoomActionsView/index.tsxapp/lib/database/facade/Model.tsapp/lib/database/facade/Collection.tsapp/lib/database/driver/connection.tsapp/lib/database/facade/__tests__/facade.test.tsapp/views/RoomView/index.tsxapp/views/NewServerView/hooks/useServersHistory.tsxapp/views/RoomView/UploadProgress.tsxapp/containers/MessageComposer/MessageComposer.tsx
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier formatting with tabs, single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses
Files:
app/definitions/ISettings.tsapp/definitions/IThread.tsapp/definitions/IPermission.tsapp/lib/database/model/CustomEmoji.jsapp/definitions/IUpload.tsapp/lib/database/model/Room.jsapp/definitions/IEmoji.tsapp/lib/database/interfaces.tsapp/lib/database/model/FrequentlyUsedEmoji.jsapp/lib/database/model/Role.jsapp/lib/database/model/servers/User.jsapp/lib/database/model/SlashCommand.jsapp/lib/database/model/Setting.jsapp/lib/database/model/Permission.jsapp/definitions/IThreadMessage.tsapp/lib/database/services/Subscription.tsapp/lib/methods/helpers/markMessagesRead.tsapp/lib/database/facade/writer.tsapp/lib/methods/getSlashCommands.tsapp/views/RoomView/List/hooks/buildVisibleSystemTypesClause.tsapp/lib/methods/getThreadName.tsapp/containers/MessageComposer/components/SendThreadToChannel.tsxapp/views/SelectServerView.tsxapp/definitions/IServerHistory.tsapp/lib/database/facade/schema.tsapp/sagas/createDiscussion.jsapp/lib/database/model/Subscription.jsapp/lib/database/schema/app.jsapp/lib/methods/getRoles.tsapp/definitions/IMessage.tsapp/lib/methods/emojis.tsapp/containers/Avatar/useAvatarETag.tsapp/lib/methods/getSettings.tsapp/sagas/messages.jsapp/views/AddExistingChannelView/index.tsxmetro.config.jsapp/containers/MessageComposer/hooks/useAutocomplete.tsapp/lib/database/model/ServersHistory.jsapp/definitions/IRoom.tsapp/definitions/ILoggedUser.tsapp/views/RoomView/List/hooks/useMessages.tsapp/definitions/IRole.tsapp/definitions/ISubscription.tsapp/lib/database/model/servers/Server.jsapp/lib/methods/sendMessage.tsapp/lib/methods/AudioManager.tsapp/views/NewMessageView/index.tsxapp/containers/MessageErrorActions.tsxbabel.config.jsapp/definitions/IServer.tsapp/lib/database/model/ThreadMessage.jsapp/views/TeamChannelsView.tsxapp/lib/database/facade/index.tsapp/lib/database/model/User.jsapp/lib/hooks/useFrequentlyUsedEmoji.tsapp/definitions/IUser.tsapp/lib/methods/helpers/findSubscriptionsRooms.tsapp/lib/database/model/Upload.jsapp/sagas/rooms.jsapp/sagas/selectServer.tsapp/lib/methods/getUsersPresence.tsapp/views/RoomsListView/hooks/useSubscriptions.tsapp/lib/database/driver/__tests__/connection.test.tsapp/views/SearchMessagesView/index.tsxapp/lib/methods/updateMessages.tsapp/lib/methods/getCustomEmojis.tsapp/views/RoomInfoEditView/hooks/useRoomDeletionActions.tsxapp/lib/methods/search.tsapp/lib/database/facade/Query.tsapp/definitions/ISlashCommand.tsapp/lib/database/facade/translate.tsapp/views/ThreadMessagesView/index.tsxapp/lib/database/model/Thread.jsapp/lib/methods/subscriptions/rooms.tsapp/lib/methods/sendFileMessage/sendFileMessage.tsapp/lib/methods/getPermissions.tsapp/lib/database/tableMaps.tsapp/lib/methods/handleMediaDownload.tsapp/lib/methods/subscriptions/room.tsapp/sagas/createChannel.jspackage.jsonapp/lib/methods/loadThreadMessages.tsapp/lib/methods/sendFileMessage/utils.tsapp/views/RoomMembersView/helpers.tsapp/lib/database/model/Message.jsapp/sagas/login.jsapp/lib/methods/logout.tsapp/views/ShareView/index.tsxapp/lib/database/facade/Q.tsapp/lib/database/facade/Database.tsapp/lib/encryption/encryption.tsapp/views/SelectedUsersView/index.tsxapp/views/ShareListView/index.tsxapp/lib/database/index.tsapp/lib/database/facade/decorators.tsapp/lib/services/connect.tsapp/lib/database/facade/observe.tsapp/views/RoomActionsView/index.tsxapp/lib/database/facade/Model.tsapp/lib/database/facade/Collection.tsapp/lib/database/driver/connection.tsapp/lib/database/facade/__tests__/facade.test.tsapp/views/RoomView/index.tsxapp/views/NewServerView/hooks/useServersHistory.tsxapp/lib/database/schema/servers.jsapp/views/RoomView/UploadProgress.tsxapp/containers/MessageComposer/MessageComposer.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Enforce ESLint rules from
@rocket.chat/eslint-configwith React, React Native, TypeScript, and Jest plugins
Files:
app/definitions/ISettings.tsapp/definitions/IThread.tsapp/definitions/IPermission.tsapp/lib/database/model/CustomEmoji.jsapp/definitions/IUpload.tsapp/lib/database/model/Room.jsapp/definitions/IEmoji.tsapp/lib/database/interfaces.tsapp/lib/database/model/FrequentlyUsedEmoji.jsapp/lib/database/model/Role.jsapp/lib/database/model/servers/User.jsapp/lib/database/model/SlashCommand.jsapp/lib/database/model/Setting.jsapp/lib/database/model/Permission.jsapp/definitions/IThreadMessage.tsapp/lib/database/services/Subscription.tsapp/lib/methods/helpers/markMessagesRead.tsapp/lib/database/facade/writer.tsapp/lib/methods/getSlashCommands.tsapp/views/RoomView/List/hooks/buildVisibleSystemTypesClause.tsapp/lib/methods/getThreadName.tsapp/containers/MessageComposer/components/SendThreadToChannel.tsxapp/views/SelectServerView.tsxapp/definitions/IServerHistory.tsapp/lib/database/facade/schema.tsapp/sagas/createDiscussion.jsapp/lib/database/model/Subscription.jsapp/lib/database/schema/app.jsapp/lib/methods/getRoles.tsapp/definitions/IMessage.tsapp/lib/methods/emojis.tsapp/containers/Avatar/useAvatarETag.tsapp/lib/methods/getSettings.tsapp/sagas/messages.jsapp/views/AddExistingChannelView/index.tsxmetro.config.jsapp/containers/MessageComposer/hooks/useAutocomplete.tsapp/lib/database/model/ServersHistory.jsapp/definitions/IRoom.tsapp/definitions/ILoggedUser.tsapp/views/RoomView/List/hooks/useMessages.tsapp/definitions/IRole.tsapp/definitions/ISubscription.tsapp/lib/database/model/servers/Server.jsapp/lib/methods/sendMessage.tsapp/lib/methods/AudioManager.tsapp/views/NewMessageView/index.tsxapp/containers/MessageErrorActions.tsxbabel.config.jsapp/definitions/IServer.tsapp/lib/database/model/ThreadMessage.jsapp/views/TeamChannelsView.tsxapp/lib/database/facade/index.tsapp/lib/database/model/User.jsapp/lib/hooks/useFrequentlyUsedEmoji.tsapp/definitions/IUser.tsapp/lib/methods/helpers/findSubscriptionsRooms.tsapp/lib/database/model/Upload.jsapp/sagas/rooms.jsapp/sagas/selectServer.tsapp/lib/methods/getUsersPresence.tsapp/views/RoomsListView/hooks/useSubscriptions.tsapp/lib/database/driver/__tests__/connection.test.tsapp/views/SearchMessagesView/index.tsxapp/lib/methods/updateMessages.tsapp/lib/methods/getCustomEmojis.tsapp/views/RoomInfoEditView/hooks/useRoomDeletionActions.tsxapp/lib/methods/search.tsapp/lib/database/facade/Query.tsapp/definitions/ISlashCommand.tsapp/lib/database/facade/translate.tsapp/views/ThreadMessagesView/index.tsxapp/lib/database/model/Thread.jsapp/lib/methods/subscriptions/rooms.tsapp/lib/methods/sendFileMessage/sendFileMessage.tsapp/lib/methods/getPermissions.tsapp/lib/database/tableMaps.tsapp/lib/methods/handleMediaDownload.tsapp/lib/methods/subscriptions/room.tsapp/sagas/createChannel.jsapp/lib/methods/loadThreadMessages.tsapp/lib/methods/sendFileMessage/utils.tsapp/views/RoomMembersView/helpers.tsapp/lib/database/model/Message.jsapp/sagas/login.jsapp/lib/methods/logout.tsapp/views/ShareView/index.tsxapp/lib/database/facade/Q.tsapp/lib/database/facade/Database.tsapp/lib/encryption/encryption.tsapp/views/SelectedUsersView/index.tsxapp/views/ShareListView/index.tsxapp/lib/database/index.tsapp/lib/database/facade/decorators.tsapp/lib/services/connect.tsapp/lib/database/facade/observe.tsapp/views/RoomActionsView/index.tsxapp/lib/database/facade/Model.tsapp/lib/database/facade/Collection.tsapp/lib/database/driver/connection.tsapp/lib/database/facade/__tests__/facade.test.tsapp/views/RoomView/index.tsxapp/views/NewServerView/hooks/useServersHistory.tsxapp/lib/database/schema/servers.jsapp/views/RoomView/UploadProgress.tsxapp/containers/MessageComposer/MessageComposer.tsx
app/definitions/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place shared TypeScript type definitions in 'app/definitions/' directory
Files:
app/definitions/ISettings.tsapp/definitions/IThread.tsapp/definitions/IPermission.tsapp/definitions/IUpload.tsapp/definitions/IEmoji.tsapp/definitions/IThreadMessage.tsapp/definitions/IServerHistory.tsapp/definitions/IMessage.tsapp/definitions/IRoom.tsapp/definitions/ILoggedUser.tsapp/definitions/IRole.tsapp/definitions/ISubscription.tsapp/definitions/IServer.tsapp/definitions/IUser.tsapp/definitions/ISlashCommand.ts
app/views/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place screen components in 'app/views/' directory
Files:
app/views/RoomView/List/hooks/buildVisibleSystemTypesClause.tsapp/views/SelectServerView.tsxapp/views/AddExistingChannelView/index.tsxapp/views/RoomView/List/hooks/useMessages.tsapp/views/NewMessageView/index.tsxapp/views/TeamChannelsView.tsxapp/views/RoomsListView/hooks/useSubscriptions.tsapp/views/SearchMessagesView/index.tsxapp/views/RoomInfoEditView/hooks/useRoomDeletionActions.tsxapp/views/ThreadMessagesView/index.tsxapp/views/RoomMembersView/helpers.tsapp/views/ShareView/index.tsxapp/views/SelectedUsersView/index.tsxapp/views/ShareListView/index.tsxapp/views/RoomActionsView/index.tsxapp/views/RoomView/index.tsxapp/views/NewServerView/hooks/useServersHistory.tsxapp/views/RoomView/UploadProgress.tsx
app/containers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place reusable UI components in 'app/containers/' directory
Files:
app/containers/MessageComposer/components/SendThreadToChannel.tsxapp/containers/Avatar/useAvatarETag.tsapp/containers/MessageComposer/hooks/useAutocomplete.tsapp/containers/MessageErrorActions.tsxapp/containers/MessageComposer/MessageComposer.tsx
app/sagas/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place sagas in 'app/sagas/' directory for handling side effects
Files:
app/sagas/selectServer.ts
app/lib/encryption/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place end-to-end encryption implementation in 'app/lib/encryption/' directory using
@rocket.chat/mobile-crypto
Files:
app/lib/encryption/encryption.ts
app/lib/services/connect.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use 'app/lib/services/connect.ts' for server connection management
Files:
app/lib/services/connect.ts
🧠 Learnings (4)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/definitions/ISettings.tsapp/definitions/IThread.tsapp/definitions/IPermission.tsapp/definitions/IUpload.tsapp/definitions/IEmoji.tsapp/lib/database/interfaces.tsapp/definitions/IThreadMessage.tsapp/lib/database/services/Subscription.tsapp/lib/methods/helpers/markMessagesRead.tsapp/lib/database/facade/writer.tsapp/lib/methods/getSlashCommands.tsapp/views/RoomView/List/hooks/buildVisibleSystemTypesClause.tsapp/lib/methods/getThreadName.tsapp/containers/MessageComposer/components/SendThreadToChannel.tsxapp/views/SelectServerView.tsxapp/definitions/IServerHistory.tsapp/lib/database/facade/schema.tsapp/lib/methods/getRoles.tsapp/definitions/IMessage.tsapp/lib/methods/emojis.tsapp/containers/Avatar/useAvatarETag.tsapp/lib/methods/getSettings.tsapp/views/AddExistingChannelView/index.tsxapp/containers/MessageComposer/hooks/useAutocomplete.tsapp/definitions/IRoom.tsapp/definitions/ILoggedUser.tsapp/views/RoomView/List/hooks/useMessages.tsapp/definitions/IRole.tsapp/definitions/ISubscription.tsapp/lib/methods/sendMessage.tsapp/lib/methods/AudioManager.tsapp/views/NewMessageView/index.tsxapp/containers/MessageErrorActions.tsxapp/definitions/IServer.tsapp/views/TeamChannelsView.tsxapp/lib/database/facade/index.tsapp/lib/hooks/useFrequentlyUsedEmoji.tsapp/definitions/IUser.tsapp/lib/methods/helpers/findSubscriptionsRooms.tsapp/sagas/selectServer.tsapp/lib/methods/getUsersPresence.tsapp/views/RoomsListView/hooks/useSubscriptions.tsapp/lib/database/driver/__tests__/connection.test.tsapp/views/SearchMessagesView/index.tsxapp/lib/methods/updateMessages.tsapp/lib/methods/getCustomEmojis.tsapp/views/RoomInfoEditView/hooks/useRoomDeletionActions.tsxapp/lib/methods/search.tsapp/lib/database/facade/Query.tsapp/definitions/ISlashCommand.tsapp/lib/database/facade/translate.tsapp/views/ThreadMessagesView/index.tsxapp/lib/methods/subscriptions/rooms.tsapp/lib/methods/sendFileMessage/sendFileMessage.tsapp/lib/methods/getPermissions.tsapp/lib/database/tableMaps.tsapp/lib/methods/handleMediaDownload.tsapp/lib/methods/subscriptions/room.tsapp/lib/methods/loadThreadMessages.tsapp/lib/methods/sendFileMessage/utils.tsapp/views/RoomMembersView/helpers.tsapp/lib/methods/logout.tsapp/views/ShareView/index.tsxapp/lib/database/facade/Q.tsapp/lib/database/facade/Database.tsapp/lib/encryption/encryption.tsapp/views/SelectedUsersView/index.tsxapp/views/ShareListView/index.tsxapp/lib/database/index.tsapp/lib/database/facade/decorators.tsapp/lib/services/connect.tsapp/lib/database/facade/observe.tsapp/views/RoomActionsView/index.tsxapp/lib/database/facade/Model.tsapp/lib/database/facade/Collection.tsapp/lib/database/driver/connection.tsapp/lib/database/facade/__tests__/facade.test.tsapp/views/RoomView/index.tsxapp/views/NewServerView/hooks/useServersHistory.tsxapp/views/RoomView/UploadProgress.tsxapp/containers/MessageComposer/MessageComposer.tsx
📚 Learning: 2026-05-07T13:19:52.152Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7304
File: app/sagas/deepLinking.js:237-243
Timestamp: 2026-05-07T13:19:52.152Z
Learning: In this codebase’s Redux-Saga usage, remember that `yield put(action)` dispatches through the Redux store synchronously, and any saga(s) that synchronously react via action listeners (and synchronous `put` chains) will run to completion before the calling saga resumes at its next `yield`. As a result, within a single saga there is no scheduler interleaving between a `yield select(...)` and a subsequent `yield take(...)` at the next `yield` point, so a check-then-take pattern like `const state = yield select(...); if (state !== TARGET) { yield take(a => a.type === TARGET); }` is safe from TOCTOU races under the synchronous `put`/take model described above.
Applied to files:
app/sagas/createDiscussion.jsapp/sagas/messages.jsapp/sagas/rooms.jsapp/sagas/createChannel.jsapp/sagas/login.js
📚 Learning: 2026-02-05T13:55:00.974Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6930
File: package.json:101-101
Timestamp: 2026-02-05T13:55:00.974Z
Learning: In this repository, the dependency on react-native-image-crop-picker should reference the RocketChat fork (RocketChat/react-native-image-crop-picker) with explicit commit pins, not the upstream ivpusic/react-native-image-crop-picker. Update package.json dependencies (and any lockfile) to point to the fork URL and a specific commit, ensuring edge-to-edge Android fixes are included. This pattern should apply to all package.json files in the repo that declare this dependency.
Applied to files:
package.json
📚 Learning: 2026-05-07T17:47:14.516Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7303
File: package.json:5-5
Timestamp: 2026-05-07T17:47:14.516Z
Learning: When reviewing pnpm `packageManager` version pins in any `package.json` (e.g., `"packageManager": "pnpm@<version>"`), don’t rely solely on web-search results to determine whether a version exists. For very recently published versions, cross-check the target version against the official pnpm release page (https://github.com/pnpm/pnpm/releases) and the npm registry page for pnpm (https://www.npmjs.com/package/pnpm) before flagging the pinned version as non-existent.
Applied to files:
package.json
🔇 Additional comments (86)
.eslintrc.js (2)
1-18: LGTM!
188-201: LGTM!babel.config.js (1)
11-13: LGTM!metro.config.js (1)
9-9: LGTM!package.json (1)
135-135: Both dependencies are already at their latest stable versions.RxJS 7.8.2 is the latest stable release; newer versions (8.0.0-alpha.x) are pre-release. babel-plugin-inline-import@^3.0.0 already includes version 3.0.0, the latest stable release. No updates needed.
app/lib/database/tableMaps.ts (1)
65-107: LGTM!app/lib/database/facade/index.ts (1)
8-25: LGTM!app/lib/database/driver/__tests__/connection.test.ts (1)
45-79: LGTM!Also applies to: 190-199
app/lib/database/interfaces.ts (1)
1-72: LGTM!app/lib/database/facade/__tests__/facade.test.ts (1)
1-363: LGTM!app/lib/database/model/SlashCommand.js (1)
1-1: LGTM!app/lib/database/model/Upload.js (1)
1-1: LGTM!app/lib/methods/sendFileMessage/sendFileMessage.ts (1)
4-4: LGTM!app/lib/methods/sendFileMessage/utils.ts (1)
5-5: LGTM!app/views/SelectedUsersView/index.tsx (1)
9-9: LGTM!app/views/ShareListView/index.tsx (1)
10-10: LGTM!app/views/ShareView/index.tsx (1)
8-8: LGTM!app/lib/database/model/CustomEmoji.js (1)
1-1: LGTM!app/lib/database/model/Permission.js (1)
1-1: LGTM!app/containers/MessageComposer/hooks/useAutocomplete.ts (1)
3-3: LGTM!app/lib/database/services/Subscription.ts (1)
1-1: LGTM!app/lib/methods/emojis.ts (1)
1-1: LGTM!app/views/NewMessageView/index.tsx (1)
7-7: LGTM!app/views/NewServerView/hooks/useServersHistory.tsx (1)
3-3: LGTM!app/views/RoomActionsView/index.tsx (1)
10-10: LGTM!app/lib/database/model/Message.js (1)
1-1: LGTM!app/lib/database/model/Room.js (1)
1-1: LGTM!app/lib/database/model/Setting.js (1)
1-1: LGTM!app/lib/database/model/Subscription.js (1)
1-1: LGTM!app/lib/methods/AudioManager.ts (1)
3-3: LGTM!app/lib/methods/getThreadName.ts (1)
1-1: LGTM!app/lib/methods/handleMediaDownload.ts (1)
5-5: LGTM!app/lib/methods/subscriptions/rooms.ts (1)
4-5: LGTM!app/lib/database/model/Thread.js (1)
1-1: LGTM!app/lib/database/model/ThreadMessage.js (1)
1-1: LGTM!app/lib/database/model/User.js (1)
1-1: LGTM!app/lib/database/model/servers/Server.js (1)
1-1: LGTM!app/lib/database/model/servers/User.js (1)
1-1: LGTM!app/lib/encryption/encryption.ts (1)
18-18: LGTM!app/lib/methods/logout.ts (1)
3-3: LGTM!Also applies to: 56-57
app/lib/methods/search.ts (1)
1-1: LGTM!app/lib/database/schema/app.js (1)
1-1: LGTM!app/lib/database/schema/servers.js (1)
1-1: LGTM!app/definitions/IEmoji.ts (1)
1-1: LGTM!app/definitions/IMessage.ts (1)
3-3: LGTM!app/definitions/IPermission.ts (1)
1-1: LGTM!app/lib/methods/updateMessages.ts (1)
1-2: LGTM!app/views/RoomView/List/hooks/buildVisibleSystemTypesClause.ts (1)
1-1: LGTM!app/views/RoomView/List/hooks/useMessages.ts (1)
5-5: LGTM!app/definitions/ILoggedUser.ts (1)
1-1: LGTM!app/definitions/IServer.ts (1)
1-1: LGTM!app/definitions/ISubscription.ts (1)
1-1: LGTM!app/definitions/IUpload.ts (1)
1-1: LGTM!app/definitions/IUser.ts (1)
1-1: LGTM!app/views/RoomInfoEditView/hooks/useRoomDeletionActions.tsx (1)
5-5: LGTM!app/views/SearchMessagesView/index.tsx (1)
8-8: LGTM!app/views/SelectServerView.tsx (1)
7-7: LGTM!app/definitions/IRole.ts (1)
1-1: LGTM!app/definitions/IRoom.ts (1)
1-1: LGTM!app/definitions/IServerHistory.ts (1)
1-1: LGTM!app/definitions/ISettings.ts (1)
1-1: LGTM!app/containers/Avatar/useAvatarETag.ts (1)
4-4: LGTM!app/containers/MessageComposer/MessageComposer.tsx (1)
6-6: LGTM!app/containers/MessageComposer/components/SendThreadToChannel.tsx (1)
6-6: LGTM!app/lib/hooks/useFrequentlyUsedEmoji.ts (1)
3-3: LGTM!app/definitions/ISlashCommand.ts (1)
1-1: LGTM!app/definitions/IThread.ts (1)
3-3: LGTM!app/definitions/IThreadMessage.ts (1)
1-1: LGTM!app/lib/methods/helpers/findSubscriptionsRooms.ts (1)
1-1: LGTM!app/lib/methods/loadThreadMessages.ts (1)
3-3: LGTM!app/lib/services/connect.ts (1)
4-4: LGTM!Also applies to: 51-58
app/sagas/rooms.js (1)
2-2: LGTM!app/sagas/selectServer.ts (1)
6-6: LGTM!app/sagas/createChannel.js (1)
2-2: LGTM!app/sagas/createDiscussion.js (1)
2-2: LGTM!app/sagas/login.js (1)
2-2: LGTM!app/sagas/messages.js (1)
2-2: LGTM!app/views/AddExistingChannelView/index.tsx (1)
6-6: LGTM!app/views/RoomMembersView/helpers.ts (1)
3-3: LGTM!app/views/TeamChannelsView.tsx (1)
6-6: LGTM!app/views/ThreadMessagesView/index.tsx (1)
7-7: LGTM!app/lib/database/facade/Q.ts (1)
1-151: LGTM!app/lib/database/facade/translate.ts (1)
1-109: LGTM!app/lib/database/facade/Query.ts (1)
1-62: LGTM!app/lib/database/facade/schema.ts (1)
12-125: LGTM!app/lib/database/facade/Model.ts (1)
16-164: LGTM!
Comment on lines
+61
to
+66
| if (filter?.id) { | ||
| const idCol = columns.id; | ||
| if (idCol) { | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| q = (q as any).where(eq(idCol, filter.id)); | ||
| } |
Contributor
There was a problem hiding this comment.
Use presence check for id filter instead of truthy check.
At Line 61, if (filter?.id) drops the id predicate for falsy ids and falls back to full-table selection. This lets find(id) resolve an unrelated row instead of rejecting when id is invalid/empty.
Proposed fix
- if (filter?.id) {
+ if (filter?.id !== undefined) {
const idCol = columns.id;
if (idCol) {
// eslint-disable-next-line `@typescript-eslint/no-explicit-any`
q = (q as any).where(eq(idCol, filter.id));
}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/lib/database/facade/Collection.ts` around lines 61 - 66, The truthy check
on filter?.id at line 61 incorrectly skips the id filter when filter.id is a
valid falsy value like 0, false, or an empty string, causing the code to fall
back to full-table selection. Replace the truthy check if (filter?.id) with a
presence check that distinguishes between falsy values and undefined/null, such
as checking if ('id' in filter) or using filter?.id !== undefined, so that the
where clause with eq(idCol, filter.id) is applied regardless of whether the id
value is falsy.
Comment on lines
+79
to
+110
| (db as any).transaction(() => { | ||
| for (const model of models) { | ||
| const op: PendingOp | null = model._pendingOp; | ||
| if (!op) continue; | ||
|
|
||
| const drizzleTable = this._tableMap[model._collection.table]; | ||
| if (!drizzleTable) throw new Error(`No Drizzle table for '${model._collection.table}'`); | ||
|
|
||
| if (op === 'create') { | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| (db as any) | ||
| .insert(drizzleTable) | ||
| .values(model._raw as never) | ||
| .run(); | ||
| } else if (op === 'update') { | ||
| const { id, ...rest } = model._raw; | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| (db as any) | ||
| .update(drizzleTable) | ||
| .set(rest as never) | ||
| .where(eq((drizzleTable as never as Record<string, never>).id, id)) | ||
| .run(); | ||
| } else if (op === 'destroy') { | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| (db as any) | ||
| .delete(drizzleTable) | ||
| .where(eq((drizzleTable as never as Record<string, never>).id, model._raw.id)) | ||
| .run(); | ||
| } | ||
|
|
||
| model._pendingOp = null; | ||
| } |
Contributor
There was a problem hiding this comment.
Clear _pendingOp only after a successful transaction commit.
On Line 109, models are marked as persisted before the transaction fully succeeds. If a later statement throws, SQLite rolls back, but earlier models keep _pendingOp = null, which can silently drop intended retries.
💡 Proposed fix
batch(...args: (Model | Model[] | null | undefined)[]): Promise<void> {
const models: Model[] = [];
@@
if (models.length === 0) return Promise.resolve();
const { db } = this._handle;
+ const committed: Model[] = [];
// eslint-disable-next-line `@typescript-eslint/no-explicit-any`
(db as any).transaction(() => {
for (const model of models) {
const op: PendingOp | null = model._pendingOp;
if (!op) continue;
@@
} else if (op === 'destroy') {
@@
}
-
- model._pendingOp = null;
+ committed.push(model);
}
});
+ for (const model of committed) {
+ model._pendingOp = null;
+ }
return Promise.resolve();
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/lib/database/facade/Database.ts` around lines 79 - 110, The issue is that
model._pendingOp is being cleared inside the transaction loop at line 109, which
means if a subsequent database operation fails and the transaction rolls back,
the earlier models will already have _pendingOp set to null, causing the retry
logic to silently skip them. Move the model._pendingOp = null assignment outside
of the transaction callback function so that it only executes after the entire
transaction successfully completes, ensuring that if any operation fails and the
transaction rolls back, all models still have their _pendingOp set and can be
retried.
Comment on lines
+173
to
+183
| const col = new Collection( | ||
| (childCollection as unknown as { table: string }).table, | ||
| (childCollection as unknown as { schema: TableSchema }).schema, | ||
| (childCollection as unknown as { _drizzleTable: SQLiteTable })._drizzleTable, | ||
| (childCollection as unknown as { _handle: DbHandle })._handle, | ||
| Model | ||
| ); | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| col._db = (childCollection as any)._db; | ||
|
|
||
| const query = col.query(Q.where(association.foreignKey, model.id)); |
Contributor
There was a problem hiding this comment.
@children builds queries with base Model, which can strip child-model behavior.
At Line 178, the new Collection is constructed with Model instead of the child table’s registered model class. Rows fetched from this query can lose child-specific decorators/getters expected at call sites.
Suggested patch
- const col = new Collection(
- (childCollection as unknown as { table: string }).table,
- (childCollection as unknown as { schema: TableSchema }).schema,
- (childCollection as unknown as { _drizzleTable: SQLiteTable })._drizzleTable,
- (childCollection as unknown as { _handle: DbHandle })._handle,
- Model
- );
- // eslint-disable-next-line `@typescript-eslint/no-explicit-any`
- col._db = (childCollection as any)._db;
-
- const query = col.query(Q.where(association.foreignKey, model.id));
+ // Reuse the DB-registered collection so model-class mapping stays intact.
+ // eslint-disable-next-line `@typescript-eslint/no-explicit-any`
+ const col = model._collection._db.get(childTable) as any;
+ const query = col.query(Q.where(association.foreignKey, model.id));
cache[childTable] = query;
return query;📝 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.
Suggested change
| const col = new Collection( | |
| (childCollection as unknown as { table: string }).table, | |
| (childCollection as unknown as { schema: TableSchema }).schema, | |
| (childCollection as unknown as { _drizzleTable: SQLiteTable })._drizzleTable, | |
| (childCollection as unknown as { _handle: DbHandle })._handle, | |
| Model | |
| ); | |
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | |
| col._db = (childCollection as any)._db; | |
| const query = col.query(Q.where(association.foreignKey, model.id)); | |
| // Reuse the DB-registered collection so model-class mapping stays intact. | |
| // eslint-disable-next-line `@typescript-eslint/no-explicit-any` | |
| const col = model._collection._db.get(childTable) as any; | |
| const query = col.query(Q.where(association.foreignKey, model.id)); | |
| cache[childTable] = query; | |
| return query; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/lib/database/facade/decorators.ts` around lines 173 - 183, The Collection
constructor in the `@children` decorator is being passed the generic Model class
instead of the child table's specific model class, which causes rows to lose
child-specific decorators and getters. Extract the actual model class from the
childCollection (similar to how the table, schema, _drizzleTable, and _handle
are being extracted via type assertions) and pass that model class as the final
argument to the Collection constructor instead of the generic Model parameter.
Comment on lines
+242
to
+249
| return observeRow(_handle, relationTableName, () => { | ||
| const id = model._getRaw(columnName) as string | null; | ||
| if (!id) return null; | ||
| const col = model.collections.get(relationTableName); | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const rows = (col as any)._fetchSync({ id }); | ||
| return rows.length > 0 ? rows[0] : null; | ||
| }) as Observable<T | null>; |
Contributor
There was a problem hiding this comment.
Relation.observe() cannot emit null when relation is removed/deleted.
Line 242 uses observeRow, which suppresses null results. That means clearing the FK or deleting the related row won’t emit null, despite the declared Observable<T | null> contract.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/lib/database/facade/decorators.ts` around lines 242 - 249, The
`observeRow` function used in the return statement suppresses null values, but
the declared return type `Observable<T | null>` indicates the observable should
emit null when relations are removed or deleted. Replace the use of `observeRow`
with an alternative observation mechanism that preserves null emissions,
ensuring that when the foreign key is cleared or the related row is deleted, the
observer receives a null value as expected by the contract defined in the return
type signature.
Comment on lines
+73
to
+82
| const emit = () => { | ||
| if (subscriber.closed) return; | ||
| const fresh = fetchFn(); | ||
| const shared = structuralShare(prevMap, fresh); | ||
| prevMap.clear(); | ||
| for (const row of shared) { | ||
| prevMap.set(row.id, row); | ||
| } | ||
| subscriber.next(shared); | ||
| }; |
Contributor
There was a problem hiding this comment.
Route fetch/emit failures through subscriber.error in all emit paths.
At Line 73-82, Line 108-112, and Line 143-157, fetchFn() exceptions are unguarded. When triggered by setTimeout, this can bypass RxJS error handling and surface as unhandled runtime exceptions.
Proposed fix
const emit = () => {
if (subscriber.closed) return;
- const fresh = fetchFn();
- const shared = structuralShare(prevMap, fresh);
- prevMap.clear();
- for (const row of shared) {
- prevMap.set(row.id, row);
- }
- subscriber.next(shared);
+ try {
+ const fresh = fetchFn();
+ const shared = structuralShare(prevMap, fresh);
+ prevMap.clear();
+ for (const row of shared) {
+ prevMap.set(row.id, row);
+ }
+ subscriber.next(shared);
+ } catch (e) {
+ subscriber.error(e);
+ }
}; const emit = () => {
if (subscriber.closed) return;
- const row = fetchFn();
- if (row !== null) subscriber.next(row);
+ try {
+ const row = fetchFn();
+ if (row !== null) subscriber.next(row);
+ } catch (e) {
+ subscriber.error(e);
+ }
}; const emit = (force = false) => {
if (subscriber.closed) return;
- const fresh = fetchFn();
- const shared = structuralShare(prevMap, fresh);
+ try {
+ const fresh = fetchFn();
+ const shared = structuralShare(prevMap, fresh);
- // Diff on watched columns only
- if (!force && sameByColumns(lastRows, shared, colSet)) return;
+ // Diff on watched columns only
+ if (!force && sameByColumns(lastRows, shared, colSet)) return;
- prevMap.clear();
- for (const row of shared) {
- prevMap.set(row.id, row);
- }
- lastRows = shared;
- subscriber.next(shared);
+ prevMap.clear();
+ for (const row of shared) {
+ prevMap.set(row.id, row);
+ }
+ lastRows = shared;
+ subscriber.next(shared);
+ } catch (e) {
+ subscriber.error(e);
+ }
};Also applies to: 108-112, 143-157
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/lib/database/facade/observe.ts` around lines 73 - 82, Wrap the fetchFn()
calls in try-catch blocks throughout the file to properly handle exceptions
through RxJS error handling. In the emit function (starting at line 73), wrap
the const fresh = fetchFn() call in a try-catch block, and in the catch clause,
route the error to subscriber.error() before returning. Apply the same pattern
to the other two emit paths mentioned (around lines 108-112 and 143-157) to
ensure all fetchFn() exceptions are properly caught and passed through
subscriber.error() instead of propagating as unhandled runtime exceptions when
triggered by setTimeout callbacks.
Comment on lines
+177
to
+184
| function sameByColumns<T extends HasId>(prev: T[], next: T[], cols: Set<string>): boolean { | ||
| if (prev.length !== next.length) return false; | ||
| for (let i = 0; i < prev.length; i++) { | ||
| const a = rowData(prev[i]); | ||
| const b = rowData(next[i]); | ||
| for (const col of cols) { | ||
| if (a[col] !== b[col]) return false; | ||
| } |
Contributor
There was a problem hiding this comment.
Preserve row identity checks in sameByColumns to avoid missed emissions.
At Line 177-184, the comparison only checks watched column values and array length. If rows reorder or a different row with identical watched values occupies the same index, observeWithColumns() incorrectly suppresses an update.
Proposed fix
function sameByColumns<T extends HasId>(prev: T[], next: T[], cols: Set<string>): boolean {
if (prev.length !== next.length) return false;
for (let i = 0; i < prev.length; i++) {
+ if (prev[i].id !== next[i].id) return false;
const a = rowData(prev[i]);
const b = rowData(next[i]);
for (const col of cols) {
if (a[col] !== b[col]) return false;
}
}
return true;
}📝 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.
Suggested change
| function sameByColumns<T extends HasId>(prev: T[], next: T[], cols: Set<string>): boolean { | |
| if (prev.length !== next.length) return false; | |
| for (let i = 0; i < prev.length; i++) { | |
| const a = rowData(prev[i]); | |
| const b = rowData(next[i]); | |
| for (const col of cols) { | |
| if (a[col] !== b[col]) return false; | |
| } | |
| function sameByColumns<T extends HasId>(prev: T[], next: T[], cols: Set<string>): boolean { | |
| if (prev.length !== next.length) return false; | |
| for (let i = 0; i < prev.length; i++) { | |
| if (prev[i].id !== next[i].id) return false; | |
| const a = rowData(prev[i]); | |
| const b = rowData(next[i]); | |
| for (const col of cols) { | |
| if (a[col] !== b[col]) return false; | |
| } | |
| } | |
| return true; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/lib/database/facade/observe.ts` around lines 177 - 184, The sameByColumns
function only compares watched column values at matching array indices without
checking row identity. If rows reorder or a different row with identical watched
column values replaces a row at the same index, the function incorrectly returns
true and suppresses necessary update emissions. Add a row identity check before
comparing column values in sameByColumns to ensure that even if two rows have
matching watched column values, they are still treated as different if they have
different ids. Compare the row identities first (using the id property from
HasId) to catch reordered or replaced rows.
Comment on lines
+19
to
+24
| this._tail = this._tail | ||
| .then(() => fn().then(resolve, reject)) | ||
| .then( | ||
| () => undefined, | ||
| () => undefined | ||
| ); |
Contributor
There was a problem hiding this comment.
Handle synchronous exceptions in enqueue so caller promises always settle.
At Line 20, fn() executes inside a .then callback. If fn throws before returning a promise, result is never resolved/rejected and the caller can hang indefinitely.
Suggested patch
this._tail = this._tail
- .then(() => fn().then(resolve, reject))
+ .then(async () => {
+ try {
+ resolve(await fn());
+ } catch (e) {
+ reject(e);
+ }
+ })
.then(
() => undefined,
() => undefined
);📝 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.
Suggested change
| this._tail = this._tail | |
| .then(() => fn().then(resolve, reject)) | |
| .then( | |
| () => undefined, | |
| () => undefined | |
| ); | |
| this._tail = this._tail | |
| .then(async () => { | |
| try { | |
| resolve(await fn()); | |
| } catch (e) { | |
| reject(e); | |
| } | |
| }) | |
| .then( | |
| () => undefined, | |
| () => undefined | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/lib/database/facade/writer.ts` around lines 19 - 24, The enqueue method
fails to handle synchronous exceptions thrown by fn(). When fn() throws
synchronously before returning a promise, the second .then() handler with dual
undefined callbacks catches the error and resolves to undefined instead of
rejecting the caller's promise, causing resolve and reject to never be invoked.
Wrap the fn() call in a try-catch block within the first .then() callback to
catch synchronous exceptions and call reject(error) when they occur, ensuring
the caller's promise always settles regardless of whether fn() throws
synchronously or asynchronously.
Comment on lines
+60
to
+63
| setActiveDB = async (database = ''): Promise<void> => { | ||
| const handle = await openServerDb(database); | ||
| this.databases.activeDB = new Database(handle, appSchema, appTableMap, appModelMap) as unknown as TAppDatabase; | ||
| }; |
Contributor
There was a problem hiding this comment.
Guard setActiveDB() against out-of-order async completion.
If callers trigger server switches quickly, a slower earlier openServerDb() can resolve last and overwrite activeDB with stale state.
💡 Proposed fix
class DB {
databases: IDatabases = {};
+ private _activeSwitchToken = 0;
@@
setActiveDB = async (database = ''): Promise<void> => {
+ const token = ++this._activeSwitchToken;
const handle = await openServerDb(database);
+ if (token !== this._activeSwitchToken) {
+ return;
+ }
this.databases.activeDB = new Database(handle, appSchema, appTableMap, appModelMap) as unknown as TAppDatabase;
};
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/lib/database/index.ts` around lines 60 - 63, The `setActiveDB()` method
has a race condition where multiple rapid calls can result in out-of-order async
completion, causing stale database state to overwrite newer state. Add a request
identifier (such as a counter) that increments each time `setActiveDB()` is
invoked, capture this identifier in the async operation, and only update
`this.databases.activeDB` when the returned handle belongs to the most recent
request. This ensures that slower earlier requests do not overwrite more recent
database switches.
Comment on lines
+73
to
79
| // New encrypted DBs live in a `SQLite/` subdirectory, isolated from the legacy | ||
| // plaintext WatermelonDB files at the container root. Must stay in lockstep with | ||
| // `DB_SUBDIRECTORY` / `resolveDbDirectory()` in the JS driver (connection.ts). | ||
| let sqliteDir = (groupRoot as NSString).appendingPathComponent("SQLite") | ||
| let path = (sqliteDir as NSString).appendingPathComponent(dbName) | ||
|
|
||
| guard sqlite3_open(path, &db) == SQLITE_OK else { |
Contributor
There was a problem hiding this comment.
Create the SQLite/ directory before calling sqlite3_open.
Line 79 can fail on fresh installs/upgrades if the subdirectory is missing, because this code assumes JS already created it.
💡 Proposed fix
- let sqliteDir = (groupRoot as NSString).appendingPathComponent("SQLite")
- let path = (sqliteDir as NSString).appendingPathComponent(dbName)
+ let sqliteDirUrl = URL(fileURLWithPath: groupRoot, isDirectory: true).appendingPathComponent("SQLite", isDirectory: true)
+ do {
+ try FileManager.default.createDirectory(at: sqliteDirUrl, withIntermediateDirectories: true)
+ } catch {
+ NSLog("[Database] Failed to create SQLite directory for %@: %@", dbName, String(describing: error))
+ return
+ }
+ let path = sqliteDirUrl.appendingPathComponent(dbName).path📝 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.
Suggested change
| // New encrypted DBs live in a `SQLite/` subdirectory, isolated from the legacy | |
| // plaintext WatermelonDB files at the container root. Must stay in lockstep with | |
| // `DB_SUBDIRECTORY` / `resolveDbDirectory()` in the JS driver (connection.ts). | |
| let sqliteDir = (groupRoot as NSString).appendingPathComponent("SQLite") | |
| let path = (sqliteDir as NSString).appendingPathComponent(dbName) | |
| guard sqlite3_open(path, &db) == SQLITE_OK else { | |
| // New encrypted DBs live in a `SQLite/` subdirectory, isolated from the legacy | |
| // plaintext WatermelonDB files at the container root. Must stay in lockstep with | |
| // `DB_SUBDIRECTORY` / `resolveDbDirectory()` in the JS driver (connection.ts). | |
| let sqliteDirUrl = URL(fileURLWithPath: groupRoot, isDirectory: true).appendingPathComponent("SQLite", isDirectory: true) | |
| do { | |
| try FileManager.default.createDirectory(at: sqliteDirUrl, withIntermediateDirectories: true) | |
| } catch { | |
| NSLog("[Database] Failed to create SQLite directory for %@: %@", dbName, String(describing: error)) | |
| return | |
| } | |
| let path = sqliteDirUrl.appendingPathComponent(dbName).path | |
| guard sqlite3_open(path, &db) == SQLITE_OK else { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ios/Shared/RocketChat/Database.swift` around lines 73 - 79, The sqlite3_open
call assumes the SQLite/ subdirectory already exists, but on fresh installs or
upgrades this directory may not be present, causing the operation to fail.
Before calling sqlite3_open with the path variable, create the sqliteDir
directory using FileManager.default.createDirectory with
withIntermediateDirectories set to true. Add appropriate error handling to
ensure the directory creation succeeds before attempting to open the database
connection.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
Replaces the app's direct
@nozbe/watermelondbusage with the WatermelonDB-shaped facade over the Drizzle/expo-sqlite driver, so the app runs on the SQLCipher-encrypted database while every call site keeps its existing shape. Adds the table/model maps the facade needs to build eachDatabase, wires the servers and app schemas through it, and adds an ESLint rule that bans new directwatermelondbimports.WatermelonDB stays installed only as the legacy migration reader; no JS runtime path uses it anymore.
Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-1277
How to test or reproduce
TZ=UTC pnpm test— full suite passes (1632 tests).Data now lives in the encrypted database; behaviour is unchanged from the user's perspective.
Types of changes
Checklist
Further comments
Stacked on
feat/native-1276-native-readers; review/merge that first. Part of the WatermelonDB → encrypted SQLite cutover (NATIVE-1272).Summary by CodeRabbit