Electron support #68
Electron support #68
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds an Electron-based SQLite IPC layer (open/close/delete/batch) with preload and renderer bridges; refactors core to route native vs web calls; introduces TypeScript typings; updates exports; modernizes tooling/configs; revamps example app; adds an older example project; removes legacy Windows/WinRT code; tweaks Windows csproj; updates ignore files. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor UI as Renderer (UI)
participant RN as Renderer (JS)
participant Pre as Preload (contextBridge)
participant Main as Electron Main
participant DB as SQLite Handle
UI->>RN: User action (needs DB)
RN->>Pre: sqliteapi.open(options, success, error)
Pre->>Main: ipcRenderer.invoke("sqlite:open", options)
Main->>Main: Resolve path, create AsyncDatabase
alt encrypted
Main->>DB: PRAGMA key, cipher_migrate
end
Main-->>Pre: result { name, path } or error
Pre-->>RN: success(result) or error(e)
RN->>Pre: sqliteapi.backgroundExecuteSqlBatch(batch, success, error)
Pre->>Main: invoke("sqlite:backgroundExecuteSqlBatch", batch)
loop statements
Main->>DB: prepare/run/finalize
Main-->>Main: collect { qid, type, result|message }
end
Main-->>Pre: results[]
Pre-->>RN: success(results[]) or error(e)
RN->>Pre: sqliteapi.close/delete(...)
Pre->>Main: invoke("sqlite:close"/"sqlite:delete")
Main->>DB: close / remove file
Main-->>Pre: ack
Pre-->>RN: success()
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120–180 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (14)
windows/react-native-sqlcipher-storage/react-native-sqlcipher-storage.csproj (1)
163-163: EmptyPacktarget needs clarification.Defining
Packwithout any tasks just turnsmsbuild /t:Packinto a no-op that still exits successfully, which can mask the absence of real packaging steps and break any workflow expecting artifacts. Please either wire this target up to the actual packaging actions or drop it until the implementation is ready.package.json (1)
37-43: Specify minimum version constraints for peer dependencies.Using wildcard
*for peer dependencies is overly permissive and can lead to runtime incompatibilities. Consider specifying minimum versions that you've tested with, especially for Electron which has breaking changes between major versions.Example with minimum versions based on the learnings:
"peerDependencies": { - "electron": "*", - "electron-log": "*", + "electron": ">=38.0.0", + "electron-log": ">=4.4.0", "react": "*", - "react-dom": "*", + "react-dom": ">=18.0.0", "react-native": "*", - "react-native-web": "*", + "react-native-web": ">=0.18.0", "react-native-windows": "*" },Based on learnings
electron/preload.js (1)
3-49: Consider adding input validation for options parameter.The preload script directly forwards
optionsto IPC handlers without validation. If the options object contains unexpected types or malicious data, this could cause issues in the main process. Consider adding basic validation or type checking before invoking IPC.Example validation:
open: async (options, success, error) => { if (!options || typeof options !== 'object') { error(new Error('Invalid options: must be an object')); return; } // ... rest of implementation.npmrc (1)
1-1: Document and justify use oflegacy-peer-deps=true.A peer dependency conflict on react@19.2.0 prevents installation without this flag. Retain the setting, but add a note in the repository (e.g., README or inline comment) explaining why it’s required and track a plan to resolve the upstream dependency mismatch.
electron/main.js (3)
13-16: Fix JSDoc param types for IPC events.Use Electron's invoke event type; avoid importing types at runtime.
- * @param {Event} event - * @param {{name: string}} options - * @returns {Promise} + * @param {import('electron').IpcMainInvokeEvent} event + * @param {{ name: string }} options + * @returns {Promise<void>}
41-60: Align option naming for consistency ("name" vs "path").Open uses options.name as the map key, close uses options.path. It works if values match, but it’s brittle. Prefer one field consistently.
Would you like me to submit a follow-up diff to standardize on { name } across open/close/delete?
166-174: IPC hardening (scope incoming calls).Optionally restrict handlers to known windows/origins (event.senderFrame.url), and consider validating payload shapes before use.
I can add an origin check and simple schema guards if you want.
src/sqlite.core.js (7)
77-81: Use globalThis for timers (works in RN native and web).Avoid referencing window directly.
-nextTick = - window.setImmediate || - function (fun) { - window.setTimeout(fun, 0); - }; +nextTick = + (typeof globalThis.setImmediate === "function" && globalThis.setImmediate) || + function (fun) { + setTimeout(fun, 0); + };
281-316: Fix var re‑declarations in close().You declare then re‑declare mysuccess/myerror. Assign instead to satisfy linter and avoid TDZ confusion in future refactors.
- var mysuccess, myerror; + var mysuccess, myerror; ... - var mysuccess = function (t, r) { + mysuccess = function (t, r) { if (!!success) { return success(r); } }; - var myerror = function (t, e) { + myerror = function (t, e) { if (!!error) { return error(e); } else { console.log("Error handler not provided: ", e); } };
401-419: Remove unreachable code after throw in executeSql().The return after throw is never reached.
throw { message: "InvalidStateError: DOM Exception 11: This transaction is already finalized. Transactions are committed" + " after its success or failure handlers are called. If you are using a Promise to handle callbacks, be aware that" + " implementations following the A+ standard adhere to run-to-completion semantics and so Promise resolution occurs" + " on a subsequent tick and therefore after the transaction commits.", code: 11, }; - return;
408-439: Fix var re‑declarations in executeSql().Assign to already-declared vars to silence lints.
- var mysuccess = function (t, r) { + mysuccess = function (t, r) { if (!!success) { return success(r); } }; - var myerror = function (t, e) { + myerror = function (t, e) { if (!!error) { return error(e); } else { console.log("Error handler not provided: ", e); } };
616-618: Fix var re‑declaration in run() error handler.- var myerror = function (error) { + myerror = function (error) { console.log("batch execution error: ", error); };
786-797: Fix var re‑declarations in deleteDatabase().- var mysuccess = function (r) { + mysuccess = function (r) { if (!!success) { return success(r); } }; - var myerror = function (e) { + myerror = function (e) { if (!!error) { return error(e); } else { console.log("Error handler not provided: ", e); } };
105-110: Optional: gate noisy console logs behind DEBUG.Many console.log calls are unconditional. Consider using the existing DEBUG flag or a logger to reduce noise.
Do you want me to provide a small helper to wrap logs respecting SQLiteFactory.DEBUG?
Also applies to: 231-279, 281-316, 327-352, 535-631, 664-693, 771-803
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
.gitignore(1 hunks).npmrc(1 hunks)electron/main.js(1 hunks)electron/preload.js(1 hunks)electron/renderer.js(1 hunks)jsconfig.json(1 hunks)package.json(3 hunks)react-native-sqlcipher-storage.d.ts(1 hunks)src/index.js(3 hunks)src/sqlite.core.js(15 hunks)windows/react-native-sqlcipher-storage/react-native-sqlcipher-storage.csproj(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/sqlite.core.js (1)
electron/renderer.js (2)
electronAPI(1-14)electronAPI(1-14)
🪛 Biome (2.1.2)
electron/main.js
[error] 69-105: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
src/sqlite.core.js
[error] 303-303: Shouldn't redeclare 'mysuccess'. Consider to delete it or rename it.
'mysuccess' is defined here:
(lint/suspicious/noRedeclare)
[error] 308-308: Shouldn't redeclare 'myerror'. Consider to delete it or rename it.
'myerror' is defined here:
(lint/suspicious/noRedeclare)
[error] 418-418: This code will never be reached ...
... because this statement will throw an exception beforehand
(lint/correctness/noUnreachable)
[error] 426-426: Shouldn't redeclare 'mysuccess'. Consider to delete it or rename it.
'mysuccess' is defined here:
(lint/suspicious/noRedeclare)
[error] 431-431: Shouldn't redeclare 'myerror'. Consider to delete it or rename it.
'myerror' is defined here:
(lint/suspicious/noRedeclare)
[error] 616-616: Shouldn't redeclare 'myerror'. Consider to delete it or rename it.
'myerror' is defined here:
(lint/suspicious/noRedeclare)
[error] 786-786: Shouldn't redeclare 'mysuccess'. Consider to delete it or rename it.
'mysuccess' is defined here:
(lint/suspicious/noRedeclare)
[error] 791-791: Shouldn't redeclare 'myerror'. Consider to delete it or rename it.
'myerror' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (6)
.gitignore (1)
92-92: LGTM!Adding
.vscodeto.gitignoreis a standard practice to prevent IDE-specific configuration from being committed.jsconfig.json (1)
1-8: LGTM!The JavaScript project configuration correctly sets up ES6 modules with Node resolution and appropriately includes both source and Electron directories while excluding
node_modules.package.json (2)
6-8: LGTM!The addition of
browserandtypesfields correctly configures the package for bundler resolution and TypeScript support.
63-65: LGTM!The expanded
filesarray correctly includes the new Electron directory and TypeScript declarations for packaging.src/index.js (2)
116-118: LGTM!The migration from CommonJS
module.exportsto ES6export defaultmodernizes the module system and aligns with the Electron and web module patterns used elsewhere in the codebase.
34-38: LGTM!The formatting updates to use arrow functions in
forEachcallbacks improve code consistency and readability.
| "react-native": "^0.61.5", | ||
| "react-native-windows": "^0.61.0" | ||
| "react": "^18.2.0", | ||
| "react-dom": "^19.2.0", |
There was a problem hiding this comment.
Invalid React DOM version.
The version ^19.2.0 for react-dom does not exist. As of October 2025, React 18 is the latest stable version. This typo will cause installation failures.
Apply this diff to correct the version:
- "react-dom": "^19.2.0",
+ "react-dom": "^18.2.0",📝 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.
| "react-dom": "^19.2.0", | |
| "react-dom": "^18.2.0", |
🤖 Prompt for AI Agents
In package.json around line 47, the react-dom dependency is set to the
non-existent version "^19.2.0"; update that entry to a valid React 18 version
(for example "^18.2.0") so it matches the actual latest stable React release and
avoids install failures.
| errorCallback?: SQLErrorCallback | ||
| ): Promise<SuccessResult[]>; | ||
|
|
||
| transaction(callback: (tx: Transaction) => void): void; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
API design inconsistency: mixed callback and Promise patterns.
The transaction method uses a callback pattern while executeSql returns a Promise. This inconsistency can confuse users and makes the API harder to use. Consider one of these approaches:
- Make
transactionreturn a Promise that resolves when the transaction completes - Have the transaction callback return a Promise that the library awaits
- Document clearly why the mixed pattern is necessary (if it is)
Example of a more consistent Promise-based API:
export interface Database {
executeSql(
sql: string,
params?: SQLParams,
successCallback?: SQLSuccessCallback,
errorCallback?: SQLErrorCallback
): Promise<SuccessResult[]>;
- transaction(callback: (tx: Transaction) => void): void;
+ transaction(callback: (tx: Transaction) => void | Promise<void>): Promise<void>;
}📝 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.
| transaction(callback: (tx: Transaction) => void): void; | |
| transaction(callback: (tx: Transaction) => void | Promise<void>): Promise<void>; |
🤖 Prompt for AI Agents
In react-native-sqlcipher-storage.d.ts around line 37 the transaction method is
declared with a callback-only signature which is inconsistent with executeSql
returning a Promise; change the typing to a Promise-based API (e.g.,
transaction(callback: (tx: Transaction) => Promise<void>): Promise<void> or
transaction(fn: (tx: Transaction) => Promise<void>): Promise<void>) so callers
can await transaction completion, or alternatively update the declaration so
transaction returns Promise<void> and the callback may return a Promise that is
awaited; update types accordingly and adjust any related Transaction.executeSql
typings to ensure the Promise chain is preserved.
| import { electronAPI } from "../electron/renderer"; | ||
|
|
||
| var mod = {}; | ||
|
|
||
| if (Platform.OS === "web") { | ||
| mod = electronAPI; | ||
| } else { | ||
| mod = NativeModules.SQLite; | ||
| } |
There was a problem hiding this comment.
Avoid static import of electron renderer in RN native builds.
Static import may break Metro/iOS/Android builds. Lazy-require only on web.
-import { NativeModules, Platform } from "react-native";
-import { electronAPI } from "../electron/renderer";
+import { NativeModules, Platform } from "react-native";
var mod = {};
-if (Platform.OS === "web") {
- mod = electronAPI;
-} else {
- mod = NativeModules.SQLite;
-}
+if (Platform.OS === "web") {
+ // Lazy require to avoid bundling electron code on native
+ // eslint-disable-next-line global-require
+ const { electronAPI } = require("../electron/renderer");
+ mod = electronAPI;
+} else {
+ mod = NativeModules.SQLite;
+}📝 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.
| import { electronAPI } from "../electron/renderer"; | |
| var mod = {}; | |
| if (Platform.OS === "web") { | |
| mod = electronAPI; | |
| } else { | |
| mod = NativeModules.SQLite; | |
| } | |
| import { NativeModules, Platform } from "react-native"; | |
| var mod = {}; | |
| if (Platform.OS === "web") { | |
| // Lazy require to avoid bundling electron code on native | |
| // eslint-disable-next-line global-require | |
| const { electronAPI } = require("../electron/renderer"); | |
| mod = electronAPI; | |
| } else { | |
| mod = NativeModules.SQLite; | |
| } |
🤖 Prompt for AI Agents
In src/sqlite.core.js around lines 18 to 26, the file statically imports the
electron renderer which can break Metro/iOS/Android builds; change to
lazy-require the electron renderer only when Platform.OS === "web" by removing
the top-level import and using require('../electron/renderer') inside the web
branch so native builds never load that module; keep the NativeModules reference
for non-web paths and ensure Platform/NativeModules are imported from
'react-native' at the top if not already.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (9)
electron/renderer.js (1)
1-14: Return delegated results to preserve async chains.The wrapper methods call
window.sqliteapibut don't return the result. If the preload methods are async and return values, these wrappers should return the result to preserve the async chain.This issue was already identified in a previous review. Apply this diff:
export const electronAPI = { open: (options, success, error) => { - window.sqliteapi.open(options, success, error); + return window.sqliteapi.open(options, success, error); }, close: (options, success, error) => { - window.sqliteapi.close(options, success, error); + return window.sqliteapi.close(options, success, error); }, delete: (options, success, error) => { - window.sqliteapi.delete(options, success, error); + return window.sqliteapi.delete(options, success, error); }, backgroundExecuteSqlBatch: (options, success, error) => { - window.sqliteapi.backgroundExecuteSqlBatch(options, success, error); + return window.sqliteapi.backgroundExecuteSqlBatch(options, success, error); }, };electron/preload.js (2)
6-6: Replace console.log with electron-log.Production code contains debug
console.logstatements. Useelectron-log/preload(already a peer dependency) for configurable logging.This was flagged in a previous review. Replace with electron-log:
+ import log from 'electron-log/preload'; + export function initSqliteAPI() { contextBridge.exposeInMainWorld("sqliteapi", { open: async (options, success, error) => { - console.log("sqliteapi:open"); + log.debug("sqliteapi:open", options); try { const result = await ipcRenderer.invoke("sqlite:open", options); - console.log("---open", result); + log.debug("sqliteapi:open result", result); success(result); } catch (e) { - console.error(e); + log.error("sqliteapi:open error", e); error(e); } },Apply similar changes to other methods.
Also applies to: 9-9, 17-17, 20-20, 28-28, 37-37
27-34: Inconsistent success callback in delete method.The
deletemethod callssuccess()without the IPC result, while other methods passsuccess(result). This inconsistency could confuse API consumers.Apply this diff:
delete: async (options, success, error) => { console.log("sqliteapi:delete"); try { const result = await ipcRenderer.invoke("sqlite:delete", options); - success(); + success(result); } catch (e) { error(e); } },electron/main.js (4)
17-33: Critical issues with path handling and IPC response.Multiple critical issues identified in previous reviews:
- Path traversal vulnerability: Renderer-controlled
options.namecan escape userData via../or absolute paths.- Non-cloneable DB handle: Returning the sqlite3 Database object will fail structured clone over IPC.
These issues were already flagged. Key fixes needed:
- Validate and sanitize
options.nameto prevent path traversal- Return a serializable acknowledgement instead of the DB instance
- See previous review comments for detailed fix suggestions
41-60: Database key mismatch between open and close.The code stores databases using
options.nameinopen()but retrieves them usingoptions.pathinclose(). This mismatch causes lookups to fail.Already flagged in previous review. Ensure consistent key usage across all methods (open, close, delete, backgroundExecuteSqlBatch).
134-181: SELECT queries won't return rows (uses run instead of all).The
#allhelper has been updated to branch on SQL type, usingstatement.all()for SELECT. However, it should usedb.all()ordb.prepare().all()correctly.Previous review identified that
statement.run()never returns rows. Verify the current implementation at lines 140-158 correctly usesthis.all()on the prepared statement for SELECT queries, and thatthiscontext is preserved.
188-193: Allow deletion when DB is not open.The
deletemethod only deletes if the DB is in the map. Users expect deleteDatabase to remove the file regardless of whether it's currently open.Already flagged in previous review. Always attempt to delete the file, not just when
this.#databases.has(options.path)is true.package.json (1)
52-52: Invalid React DOM version.The version
^19.2.0forreact-domdoes not exist. As of October 2025, React 18 is the latest stable version.Already flagged in previous review. Apply this diff:
- "react-dom": "^19.2.0", + "react-dom": "^18.2.0",src/sqlite.core.js (1)
16-26: Static electron import breaks native builds.The static import of
electron/rendererwill be bundled in React Native builds, causing Metro/iOS/Android build failures. Use lazy require only whenPlatform.OS === "web".This was flagged in a previous review. Apply this diff:
import base64 from "base-64"; import { NativeModules, Platform } from "react-native"; -import { electronAPI } from "../electron/renderer"; var mod = {}; if (Platform.OS === "web") { + // eslint-disable-next-line global-require + const { electronAPI } = require("../electron/renderer"); mod = electronAPI; } else { mod = NativeModules.SQLite; }
🧹 Nitpick comments (2)
package.json (1)
59-60: Update base-64 to the latest version
base-64@^0.1.0is over a decade old; upgrade to^1.0.0. No known security advisories for these dependencies..prettierrc (1)
17-20: Scope Flow parser override to Flow files only. The only.flowconfigis inexample/, so narrow the override’sfilesto"example/**/*.js"(or remove the global parser override if Flow isn’t used elsewhere).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (29)
.prettierrc(1 hunks)electron/main.js(1 hunks)electron/preload.js(1 hunks)electron/renderer.js(1 hunks)eslint.config.mjs(1 hunks)package.json(3 hunks)react-native-sqlcipher-storage.d.ts(1 hunks)src/index.js(5 hunks)src/sqlite.core.js(16 hunks)windowsOld/SQLite3-WinRT/SQLite3/Common.cpp(0 hunks)windowsOld/SQLite3-WinRT/SQLite3/Common.h(0 hunks)windowsOld/SQLite3-WinRT/SQLite3/Constants.cpp(0 hunks)windowsOld/SQLite3-WinRT/SQLite3/Constants.h(0 hunks)windowsOld/SQLite3-WinRT/SQLite3/Database.cpp(0 hunks)windowsOld/SQLite3-WinRT/SQLite3/Database.h(0 hunks)windowsOld/SQLite3-WinRT/SQLite3/SQLCipher/.gitignore(0 hunks)windowsOld/SQLite3-WinRT/SQLite3/SQLCipher/SQLCipher.sln(0 hunks)windowsOld/SQLite3-WinRT/SQLite3/SQLCipher/SQLCipher/Class1.cpp(0 hunks)windowsOld/SQLite3-WinRT/SQLite3/SQLCipher/SQLCipher/Class1.h(0 hunks)windowsOld/SQLite3-WinRT/SQLite3/SQLCipher/SQLCipher/PropertySheet.props(0 hunks)windowsOld/SQLite3-WinRT/SQLite3/SQLCipher/SQLCipher/PropertySheet1.props(0 hunks)windowsOld/SQLite3-WinRT/SQLite3/SQLCipher/SQLCipher/SQLCipher.vcxproj(0 hunks)windowsOld/SQLite3-WinRT/SQLite3/SQLCipher/SQLCipher/SQLCipher.vcxproj.filters(0 hunks)windowsOld/SQLite3-WinRT/SQLite3/SQLCipher/SQLCipher/pch.cpp(0 hunks)windowsOld/SQLite3-WinRT/SQLite3/SQLCipher/SQLCipher/pch.h(0 hunks)windowsOld/SQLite3-WinRT/SQLite3/SQLiteModule.cs(0 hunks)windowsOld/SQLite3-WinRT/SQLite3/Statement.cpp(0 hunks)windowsOld/SQLite3-WinRT/SQLite3/Statement.h(0 hunks)windowsOld/SQLite3-WinRT/SQLite3JS/js/SQLite3.js(0 hunks)
💤 Files with no reviewable changes (20)
- windowsOld/SQLite3-WinRT/SQLite3/SQLCipher/SQLCipher/PropertySheet.props
- windowsOld/SQLite3-WinRT/SQLite3/SQLCipher/.gitignore
- windowsOld/SQLite3-WinRT/SQLite3/SQLCipher/SQLCipher/Class1.h
- windowsOld/SQLite3-WinRT/SQLite3/Database.cpp
- windowsOld/SQLite3-WinRT/SQLite3/Constants.h
- windowsOld/SQLite3-WinRT/SQLite3/Constants.cpp
- windowsOld/SQLite3-WinRT/SQLite3JS/js/SQLite3.js
- windowsOld/SQLite3-WinRT/SQLite3/SQLCipher/SQLCipher/SQLCipher.vcxproj.filters
- windowsOld/SQLite3-WinRT/SQLite3/SQLCipher/SQLCipher.sln
- windowsOld/SQLite3-WinRT/SQLite3/SQLCipher/SQLCipher/pch.h
- windowsOld/SQLite3-WinRT/SQLite3/SQLiteModule.cs
- windowsOld/SQLite3-WinRT/SQLite3/SQLCipher/SQLCipher/Class1.cpp
- windowsOld/SQLite3-WinRT/SQLite3/SQLCipher/SQLCipher/SQLCipher.vcxproj
- windowsOld/SQLite3-WinRT/SQLite3/SQLCipher/SQLCipher/pch.cpp
- windowsOld/SQLite3-WinRT/SQLite3/Common.h
- windowsOld/SQLite3-WinRT/SQLite3/SQLCipher/SQLCipher/PropertySheet1.props
- windowsOld/SQLite3-WinRT/SQLite3/Statement.h
- windowsOld/SQLite3-WinRT/SQLite3/Database.h
- windowsOld/SQLite3-WinRT/SQLite3/Common.cpp
- windowsOld/SQLite3-WinRT/SQLite3/Statement.cpp
🚧 Files skipped from review as they are similar to previous changes (2)
- react-native-sqlcipher-storage.d.ts
- src/index.js
🧰 Additional context used
🧬 Code graph analysis (1)
src/sqlite.core.js (2)
electron/renderer.js (2)
electronAPI(1-14)electronAPI(1-14)android/src/main/java/com/axsy/SQLitePlugin.java (1)
SQLitePlugin(52-1154)
🪛 Biome (2.1.2)
src/sqlite.core.js
[error] 303-303: Shouldn't redeclare 'mysuccess'. Consider to delete it or rename it.
'mysuccess' is defined here:
(lint/suspicious/noRedeclare)
[error] 308-308: Shouldn't redeclare 'myerror'. Consider to delete it or rename it.
'myerror' is defined here:
(lint/suspicious/noRedeclare)
[error] 418-418: This code will never be reached ...
... because this statement will throw an exception beforehand
(lint/correctness/noUnreachable)
[error] 426-426: Shouldn't redeclare 'mysuccess'. Consider to delete it or rename it.
'mysuccess' is defined here:
(lint/suspicious/noRedeclare)
[error] 431-431: Shouldn't redeclare 'myerror'. Consider to delete it or rename it.
'myerror' is defined here:
(lint/suspicious/noRedeclare)
[error] 616-616: Shouldn't redeclare 'myerror'. Consider to delete it or rename it.
'myerror' is defined here:
(lint/suspicious/noRedeclare)
[error] 786-786: Shouldn't redeclare 'mysuccess'. Consider to delete it or rename it.
'mysuccess' is defined here:
(lint/suspicious/noRedeclare)
[error] 791-791: Shouldn't redeclare 'myerror'. Consider to delete it or rename it.
'myerror' is defined here:
(lint/suspicious/noRedeclare)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (2)
electron/main.js (1)
75-125: Async Promise executor and error handling issues.The batch executor has been updated with try-catch per statement, which is good progress. However, verify that all error paths properly return error results rather than throwing.
The per-statement error handling looks better than the previous version. Verify error results are properly formatted for the renderer.
src/sqlite.core.js (1)
354-384: Formatting changes look good.The spacing and formatting updates throughout the file improve readability while preserving functionality.
eslint.config.mjs
Outdated
| import { defineConfig, globalIgnores } from "eslint/config"; | ||
|
|
||
| export default defineConfig([ | ||
| globalIgnores(["src/", "example/"]), |
There was a problem hiding this comment.
Ignoring src/ prevents linting of source code.
The configuration ignores the src/ directory, which typically contains the main source code that should be linted. This appears to be a mistake. Consider removing src/ from globalIgnores unless there's a specific reason to exclude it.
Apply this diff:
- globalIgnores(["src/", "example/"]),
+ globalIgnores(["example/"]),📝 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.
| globalIgnores(["src/", "example/"]), | |
| globalIgnores(["example/"]), |
🤖 Prompt for AI Agents
In eslint.config.mjs around line 6, the globalIgnores call currently includes
"src/" which prevents linting of the project source; remove "src/" from the
globalIgnores array (or if there is a specific reason to exclude source, replace
it with more appropriate ignore paths like build/ or dist/) so that source files
are linted, then run the linter to confirm only generated or intended artifact
directories remain ignored.
| var mysuccess = function (t, r) { | ||
| if (!!success) { | ||
| return success(r); | ||
| } | ||
| }; | ||
| var myerror = function(t, e) { | ||
| var myerror = function (t, e) { |
There was a problem hiding this comment.
Remove variable redeclarations.
The code redeclares mysuccess and myerror using var, which is flagged by the linter. These should be removed or renamed.
Apply this diff:
} else {
- console.log("closing db with no transaction lock state");
+ console.log("closing db with no transaction lock state");
}
- var mysuccess = function (t, r) {
+ mysuccess = function (t, r) {
if (!!success) {
return success(r);
}
};
- var myerror = function (t, e) {
+ myerror = function (t, e) {
if (!!error) {
return error(e);
} else {Or better, use different variable names to avoid confusion.
📝 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.
| var mysuccess = function (t, r) { | |
| if (!!success) { | |
| return success(r); | |
| } | |
| }; | |
| var myerror = function(t, e) { | |
| var myerror = function (t, e) { | |
| } else { | |
| console.log("closing db with no transaction lock state"); | |
| } | |
| mysuccess = function (t, r) { | |
| if (!!success) { | |
| return success(r); | |
| } | |
| }; | |
| myerror = function (t, e) { | |
| if (!!error) { | |
| return error(e); | |
| } else { |
🧰 Tools
🪛 Biome (2.1.2)
[error] 303-303: Shouldn't redeclare 'mysuccess'. Consider to delete it or rename it.
'mysuccess' is defined here:
(lint/suspicious/noRedeclare)
[error] 308-308: Shouldn't redeclare 'myerror'. Consider to delete it or rename it.
'myerror' is defined here:
(lint/suspicious/noRedeclare)
🤖 Prompt for AI Agents
In src/sqlite.core.js around lines 303 to 308, the functions mysuccess and
myerror are being redeclared with var which triggers linter errors; remove the
duplicate var declarations or rename the variables to unique names. Replace "var
mysuccess = ..." and "var myerror = ..." with simple assignments (mysuccess =
function...) if the intent is to reuse outer-scope variables, or choose distinct
names (e.g., localSuccess, localError) and update their usages accordingly so no
duplicate var declarations remain.
| var mysuccess = function (t, r) { | ||
| if (!!success) { | ||
| return success(t,r); | ||
| return success(t, r); | ||
| } | ||
| }; | ||
| var myerror = function(t, e) { | ||
| var myerror = function (t, e) { |
There was a problem hiding this comment.
Multiple variable redeclarations throughout the file.
The linter flags multiple redeclarations of mysuccess and myerror using var keyword at lines 426, 431, 616, 786, and 791. These create shadowing and potential confusion.
Remove the var keyword on redeclarations or use unique variable names. Example:
};
- var mysuccess = function (t, r) {
+ mysuccess = function (t, r) {
if (!!success) {
return success(t, r);
}
};
- var myerror = function (t, e) {
+ myerror = function (t, e) {
if (!!error) {
return error(e);Also applies to: 616-616, 786-791
🧰 Tools
🪛 Biome (2.1.2)
[error] 426-426: Shouldn't redeclare 'mysuccess'. Consider to delete it or rename it.
'mysuccess' is defined here:
(lint/suspicious/noRedeclare)
[error] 431-431: Shouldn't redeclare 'myerror'. Consider to delete it or rename it.
'myerror' is defined here:
(lint/suspicious/noRedeclare)
🤖 Prompt for AI Agents
In src/sqlite.core.js around lines 426-431 (and also at lines ~616, 786-791),
multiple functions redeclare the same identifiers mysuccess and myerror using
var which causes shadowing and linter errors; update each redeclaration by
either removing the var keyword when you intend to reassign the existing
variables (i.e., reuse the outer mysuccess/myerror) or rename the local
variables to unique names (or use let/const for block-scoped bindings) so there
are no duplicate var declarations; apply the same change consistently at the
other indicated line ranges.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (10)
src/sqlite.core.js (3)
16-25: Static import breaks native builds.The static import of
electronAPIfrom../electron/rendererwill cause Metro bundler to fail on iOS/Android, as it attempts to load Electron-specific code that doesn't exist in native environments.Apply this diff to lazy-load only on web:
-import { NativeModules, Platform } from "react-native"; -import { electronAPI } from "../electron/renderer"; +import { NativeModules, Platform } from "react-native"; var mod = {}; if (Platform.OS === "web") { - mod = electronAPI; + // Lazy require to avoid bundling electron code on native + // eslint-disable-next-line global-require + const { electronAPI } = require("../electron/renderer"); + mod = electronAPI; } else { mod = NativeModules.SQLite; }
302-313: Remove variable redeclarations.Redeclaring
mysuccessandmyerrorwithvartriggers linter errors and shadows outer-scope variables.Apply this diff:
- var mysuccess = function (t, r) { + mysuccess = function (t, r) { if (!!success) { return success(r); } }; - var myerror = function (t, e) { + myerror = function (t, e) { if (!!error) { return error(e); } else { console.log("Error handler not provided: ", e); } };
425-436: Remove variable redeclarations.Redeclaring
mysuccessandmyerrorwithvartriggers linter errors. This pattern repeats at lines 615, 785, and 790.Apply this diff and similar changes at lines 615, 785, and 790:
- var mysuccess = function (t, r) { + mysuccess = function (t, r) { if (!!success) { return success(t, r); } }; - var myerror = function (t, e) { + myerror = function (t, e) { if (!!error) { return error(e); } else { console.log("Error handler not provided: ", e); } };package.json (1)
51-51: Invalid React DOM version.React 19.2.0 does not exist. As of October 2025, React 18.x is the latest stable version. This will cause installation failures.
Apply this diff:
- "react-dom": "^19.2.0", + "react-dom": "^18.3.1",electron/main.js (6)
34-41: Path traversal vulnerability.Database names are user-controlled via
options.nameand used directly in path construction without validation. This allows arbitrary file access outsideuserDatavia../or absolute paths.Apply this diff to enforce containment:
open = async (event, options) => { - const openPath = resolve(join(this.#dbLocation, options.name)); + const base = resolve(this.#dbLocation); + const name = String(options?.name ?? "").trim(); + if (!name || name.includes("..") || name.includes(path.sep)) { + throw new Error("Invalid database name"); + } + const openPath = resolve(join(base, name)); + if (!openPath.startsWith(base + path.sep)) { + throw new Error("Invalid database path"); + } const db = await AsyncDatabase.newDatabase(openPath);
48-54: Key mismatch prevents proper database management.The
openmethod stores databases byoptions.name(line 40), butcloseanddeleteretrieve usingoptions.path(lines 49, 61). This mismatch causes lookups to fail.Standardize on
options.name:close = async (event, options) => { - const db = this.#databases.get(options.path); + const db = this.#databases.get(options.name); if (db) { await db.close(); - this.#databases.delete(options.path); + this.#databases.delete(options.name); } };Apply similar changes to the
deletemethod at lines 60-65.
60-65: Delete should remove file even when DB not open.Current code no-ops if the database isn't in the map. Users expect
deleteDatabaseto remove the file regardless of open state.Apply this diff:
delete = async (event, options) => { - if (this.#databases.has(options.path)) { - await this.close(event, options); - this.#deleteDatabase(options.path); + if (this.#databases.has(options.name)) { + await this.close(event, options); } + this.#deleteDatabase(options.name); };
79-129: Batch executor should return per-item errors.Throwing at line 82 when the database doesn't exist causes the entire batch to fail. The renderer expects an array of results with type: "error" for failed items.
Apply this diff:
backgroundExecuteSqlBatch = async (event, options) => { + const executes = options?.executes ?? []; const db = this.#databases.get(options.dbargs.dbname); if (!db) { - throw new Error("Database does not exist"); + return executes.map((e) => ({ + qid: e?.qid, + type: "error", + result: "Database does not exist" + })); } const results = []; - const executes = options.executes;
142-142: Locale-sensitive SQL keyword detection.
toLocaleLowerCase()is locale-dependent, which can cause incorrect SQL keyword matching across locales (e.g., Turkish locale transforms 'I' unexpectedly).Apply this diff:
- if (sql.toLocaleLowerCase().startsWith("select")) { + if (/^\s*(select|pragma|with)\b/i.test(sql || "")) { const all = await statement.all(params);
165-170: Path traversal in delete.Similar to the
openmethod,#deleteDatabaseusesdbnamewithout validation, allowing traversal.Add validation:
#deleteDatabase(dbname) { + const name = String(dbname ?? "").trim(); + if (!name || name.includes("..") || name.includes(path.sep)) { + throw new Error("Invalid database name"); + } - const dbPath = resolve(join(this.#dbLocation, dbname)); + const base = resolve(this.#dbLocation); + const dbPath = resolve(join(base, name)); + if (!dbPath.startsWith(base + path.sep)) { + throw new Error("Invalid database path"); + } if (existsSync(dbPath)) { unlinkSync(dbPath); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
electron/main.js(1 hunks)example/package.json(0 hunks)package.json(2 hunks)src/sqlite.core.js(16 hunks)
💤 Files with no reviewable changes (1)
- example/package.json
🧰 Additional context used
🧬 Code graph analysis (1)
src/sqlite.core.js (2)
electron/renderer.js (2)
electronAPI(1-14)electronAPI(1-14)android/src/main/java/com/axsy/SQLitePlugin.java (1)
SQLitePlugin(52-1154)
🪛 Biome (2.1.2)
src/sqlite.core.js
[error] 302-302: Shouldn't redeclare 'mysuccess'. Consider to delete it or rename it.
'mysuccess' is defined here:
(lint/suspicious/noRedeclare)
[error] 307-307: Shouldn't redeclare 'myerror'. Consider to delete it or rename it.
'myerror' is defined here:
(lint/suspicious/noRedeclare)
[error] 417-417: This code will never be reached ...
... because this statement will throw an exception beforehand
(lint/correctness/noUnreachable)
[error] 425-425: Shouldn't redeclare 'mysuccess'. Consider to delete it or rename it.
'mysuccess' is defined here:
(lint/suspicious/noRedeclare)
[error] 430-430: Shouldn't redeclare 'myerror'. Consider to delete it or rename it.
'myerror' is defined here:
(lint/suspicious/noRedeclare)
[error] 615-615: Shouldn't redeclare 'myerror'. Consider to delete it or rename it.
'myerror' is defined here:
(lint/suspicious/noRedeclare)
[error] 785-785: Shouldn't redeclare 'mysuccess'. Consider to delete it or rename it.
'mysuccess' is defined here:
(lint/suspicious/noRedeclare)
[error] 790-790: Shouldn't redeclare 'myerror'. Consider to delete it or rename it.
'myerror' is defined here:
(lint/suspicious/noRedeclare)
| await db.run(`PRAGMA key = '${options.key}'`); | ||
| await db.run("PRAGMA cipher_migrate"); |
There was a problem hiding this comment.
SQL injection in PRAGMA key.
String interpolation in PRAGMA key = '${options.key}' enables SQL injection. Use parameterized queries.
Unfortunately, SQLite PRAGMA statements don't support bound parameters. Instead, validate and escape the key:
if (options.key) {
- await db.run(`PRAGMA key = '${options.key}'`);
+ const key = String(options.key).replace(/'/g, "''");
+ await db.run(`PRAGMA key = '${key}'`);
await db.run("PRAGMA cipher_migrate");
}Better yet, validate that the key doesn't contain quotes:
if (options.key) {
+ if (typeof options.key !== 'string' || options.key.includes("'")) {
+ throw new Error("Invalid key format");
+ }
await db.run(`PRAGMA key = '${options.key}'`);
await db.run("PRAGMA cipher_migrate");
}📝 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.
| await db.run(`PRAGMA key = '${options.key}'`); | |
| await db.run("PRAGMA cipher_migrate"); | |
| // … inside your open logic … | |
| if (options.key) { | |
| // reject any key containing a quote | |
| if (typeof options.key !== 'string' || options.key.includes("'")) { | |
| throw new Error("Invalid key format"); | |
| } | |
| // escape any remaining single-quotes (defense-in-depth) | |
| const key = String(options.key).replace(/'/g, "''"); | |
| await db.run(`PRAGMA key = '${key}'`); | |
| await db.run("PRAGMA cipher_migrate"); | |
| } | |
| // … |
🤖 Prompt for AI Agents
In electron/main.js around lines 37-38, the PRAGMA key line uses string
interpolation which allows SQL injection; instead validate and sanitize
options.key before embedding it: ensure options.key is a non-empty string and
reject/throw if it contains single-quote, double-quote, semicolon, or other
unsafe characters (or restrict to a safe charset like [A-Za-z0-9-_]+=), and only
then construct the PRAGMA statement; if you need to support arbitrary bytes,
transform the key into a safe encoding (e.g., base64) and use that encoded value
in the PRAGMA; do not accept raw user input with quotes—either reject or escape
single quotes by doubling them before interpolation if you must embed.
There was a problem hiding this comment.
Actionable comments posted: 13
♻️ Duplicate comments (9)
react-native-sqlcipher-storage.d.ts (1)
26-26: Make transaction awaitable for API consistency.Align with Promise-based executeSql.
- transaction(callback: (tx: Transaction) => void): void; + transaction(callback: (tx: Transaction) => void | Promise<void>): Promise<void>;package.json (1)
66-66: Invalid react-dom version (already flagged).The version ^19.2.0 does not exist; use a valid 18.x version.
- "react-dom": "^19.2.0", + "react-dom": "^18.2.0",electron/main.js (7)
33-41: Unify DB key and sanitize name; prevent path traversal.Use a single key (prefer options.name) across open/close/delete/batch, and validate name to stay within userData.
- open = async (event, options) => { - const openPath = path.resolve(path.join(this.#dbLocation, options.name)); - const db = await AsyncDatabase.newDatabase(openPath); - if (options.key) { - await db.run(`PRAGMA key = '${options.key}'`); - await db.run("PRAGMA cipher_migrate"); - } - this.#databases.set(options.name, db); - }; + open = async (event, options) => { + const base = path.resolve(this.#dbLocation); + const name = String(options?.name ?? "").trim(); + if (!name || path.isAbsolute(name) || name.includes("..") || name.includes(path.sep)) { + throw new Error("Invalid database name"); + } + const openPath = path.resolve(base, name); + if (!openPath.startsWith(base + path.sep)) { + throw new Error("Invalid database path"); + } + const db = await AsyncDatabase.newDatabase(openPath); + if (options.key) { + if (typeof options.key !== "string") throw new Error("Invalid key format"); + const key = options.key.replace(/'/g, "''"); + await db.run(`PRAGMA key = '${key}'`); + await db.run("PRAGMA cipher_migrate"); + } + this.#databases.set(name, db); + };
37-39: PRAGMA key interpolation allows SQL injection.Escape or validate the key before embedding.
- if (options.key) { - await db.run(`PRAGMA key = '${options.key}'`); - await db.run("PRAGMA cipher_migrate"); - } + if (options.key) { + if (typeof options.key !== "string") throw new Error("Invalid key format"); + const key = options.key.replace(/'/g, "''"); + await db.run(`PRAGMA key = '${key}'`); + await db.run("PRAGMA cipher_migrate"); + }
48-54: close() uses a different key than open(); align on name.Lookups will fail; use options.name (fallback to path/dbname only if provided).
- close = async (event, options) => { - const db = this.#databases.get(options.path); + close = async (event, options) => { + const key = options?.name ?? options?.dbargs?.dbname ?? options?.path; + const db = this.#databases.get(key); if (db) { await db.close(); - this.#databases.delete(options.path); + this.#databases.delete(key); } };
60-65: Allow delete even when DB isn’t open; also use unified key.Proceed with file deletion regardless of map state.
- delete = async (event, options) => { - if (this.#databases.has(options.path)) { - await this.close(event, options); - this.#deleteDatabase(options.path); - } - }; + delete = async (event, options) => { + const key = options?.name ?? options?.dbargs?.dbname ?? options?.path; + if (this.#databases.has(key)) { + await this.close(event, options); + } + this.#deleteDatabase(key); + return { ok: true }; + };
79-129: Batch handler should not throw on missing DB; return per-item errors.Rejecting breaks renderer flow; always resolve an array matching executes length.
- backgroundExecuteSqlBatch = async (event, options) => { - const db = this.#databases.get(options.dbargs.dbname); - if (!db) { - throw new Error("Database does not exist"); - } - const results = []; - const executes = options.executes; - for (const e of executes) { + backgroundExecuteSqlBatch = async (event, options) => { + const key = options?.dbargs?.dbname ?? options?.name ?? options?.path; + const db = this.#databases.get(key); + const executes = options?.executes ?? []; + const results = []; + if (!db) { + for (const e of executes) { + results.push({ qid: e?.qid, type: "error", result: new Error("Database does not exist") }); + } + return results; + } + for (const e of executes) { const execute = e; const qid = execute.qid; const sql = execute.sql; const params = execute.params; let resultInfo = { qid }; try { const { rows, rowsAffected, insertId } = await this.#all( db, sql, params ); - const hasInsertId = rowsAffected > 0 && insertId !== 0; resultInfo = { ...resultInfo, type: "success", result: { rowsAffected, rows } }; - if (hasInsertId) { - resultInfo.result.insertId = insertId; - } + if (rowsAffected > 0 && insertId != null) { + resultInfo.result.insertId = insertId; + } } catch (err) { resultInfo = { ...resultInfo, type: "error", - message: err, - result: err + result: err instanceof Error ? err : new Error(String(err)) }; } results.push(resultInfo); } return results; };
135-156: Fix read/write detection and SELECT result shape; avoid locale-sensitive APIs.Use a case-insensitive regex; for reads, rowsAffected=0 and insertId undefined.
- * @returns {Promise<{rows: any[], rowsAffected: number, insertId: number}>} + * @returns {Promise<{rows: any[], rowsAffected: number, insertId?: number}>} */ async #all(db, sql, params) { const statement = await db.prepare(sql); let result; - if (sql.toLocaleLowerCase().startsWith("select")) { - const all = await statement.all(params); - result = { - rowsAffected: all.changes, - insertId: all.lastID, - rows: all.rows - }; - } else { - const all = await statement.run(params); - result = { - rowsAffected: all.changes, - insertId: all.insertId - }; - } + const isRead = /^\s*(select|pragma|with)\b/i.test(sql || ""); + if (isRead) { + const all = await statement.all(params); + result = { + rows: all.rows || [], + rowsAffected: 0, + insertId: undefined + }; + } else { + const run = await statement.run(params); + result = { + rows: [], + rowsAffected: run.changes ?? 0, + insertId: run.lastID + }; + } await statement.finalize(); return result; }
165-170: Harden delete path against traversal.Ensure deletion stays within userData.
- #deleteDatabase(dbname) { - const dbPath = path.resolve(path.join(this.#dbLocation, dbname)); + #deleteDatabase(dbname) { + const base = path.resolve(this.#dbLocation); + const name = String(dbname ?? "").trim(); + if (!name || path.isAbsolute(name) || name.includes("..") || name.includes(path.sep)) { + throw new Error("Invalid database name"); + } + const dbPath = path.resolve(base, name); + if (!dbPath.startsWith(base + path.sep)) { + throw new Error("Invalid database path"); + } if (existsSync(dbPath)) { unlinkSync(dbPath); } }
🧹 Nitpick comments (19)
example-prev/metro.config.js (2)
9-9: Update deprecated Metro API usage.The
blacklistimport andblacklistREoption have been deprecated in favor ofblockListin newer Metro versions.Apply this diff to update to the current API:
-const blacklist = require('metro-config/src/defaults/blacklist'); +const exclusionList = require('metro-config/src/defaults/exclusionList');And update the resolver configuration:
- blacklistRE: blacklist([ + blockList: exclusionList([ new RegExp(Also applies to: 52-52
11-76: Consider using @rnx-kit/metro-config to simplify this complex manual configuration.The current manual configuration includes significant complexity with path resolution, dependency mapping, and platform-specific handling. Based on retrieved learnings,
@rnx-kit/metro-config(v2.x) is designed specifically to handle these scenarios for React Native projects, especially those targeting Windows platforms.If simplification is desired, consider refactoring to use
@rnx-kit/metro-config:const { makeMetroConfig } = require('@rnx-kit/metro-config'); module.exports = makeMetroConfig({ projectRoot: __dirname, watchFolders: [path.resolve(__dirname, '..')], transformer: { getTransformOptions: async () => ({ transform: { experimentalImportSupport: false, inlineRequires: false, }, }), }, });This would automatically handle:
- Platform resolution (Windows, macOS, etc.)
- Path and dependency mapping
- React Native/React Native Windows resolution
- Exclusion patterns
Based on learnings.
example/react-native.config.js (2)
16-17: Consider logging the error for debugging.The catch block silently swallows all errors, which could hide configuration issues such as missing dependencies, invalid paths, or API changes in
react-native-test-app. Consider logging the error to aid debugging while still providing a graceful fallback.Apply this diff:
- } catch (_) { + } catch (e) { + console.warn('Failed to configure react-native-test-app project:', e.message); return undefined; }
22-22: Consider spreading an empty object for clarity.Spreading
undefinedis valid but less explicit than spreading an empty object. For improved readability, consider using{}as the fallback.Apply this diff:
- ...(project ? { project } : undefined), + ...(project ? { project } : {}),example/Gemfile (3)
8-10: Alphabetize gems and re-evaluate hard pins (potential conflicts).
- Alphabetize per Bundler/OrderedGems.
- Please confirm the necessity of pinning activesupport and xcodeproj; these can conflict with newer CocoaPods. Prefer letting CocoaPods resolve its transitive deps unless a known incompatibility exists.
Apply ordering:
-gem 'cocoapods', '>= 1.13', '< 1.15' -gem 'activesupport', '>= 6.1.7.5', '< 7.1.0' -gem 'xcodeproj', '< 1.26.0' +gem 'activesupport', '>= 6.1.7.5', '< 7.1.0' +gem 'cocoapods', '>= 1.13', '< 1.15' +gem 'xcodeproj', '< 1.26.0'If the pins are required, add brief inline rationale (issue link) to avoid future tightening/loosening churn. Based on static analysis hints.
4-4: Raise minimum Ruby version (2.6 is EOL).Recommend Ruby >= 2.7 to avoid compatibility issues with modern CocoaPods/ActiveSupport and macOS toolchains.
Apply:
-ruby ">= 2.6.10" +ruby ">= 2.7"
6-7: Fix comment casing/grammar.Small polish.
-# Cocoapods 1.15 introduced a bug which break the build. We will remove the upper -# bound in the template on Cocoapods with next React Native release. +# CocoaPods 1.15 introduced a bug which breaks the build. We will remove the upper +# bound in the template for CocoaPods with the next React Native release.example-prev/react-native.config.js (1)
1-5: Make reactNativePath resolution resilient; avoid crashes when RNW isn’t installedUnconditional require.resolve will throw in environments without react-native-windows (e.g., Android/iOS only). Guard it.
const fs = require('fs'); const path = require('path'); -module.exports = { - reactNativePath: fs.realpathSync(path.resolve(require.resolve('react-native-windows/package.json'), '..')), -}; +let rnwPath = null; +try { + rnwPath = fs.realpathSync(path.dirname(require.resolve('react-native-windows/package.json'))); +} catch (_) { + // react-native-windows not installed; leave config empty +} +module.exports = rnwPath ? { reactNativePath: rnwPath } : {};example/metro.config.js (1)
13-15: Avoid watchFolders pointing at node_modulesMetro already watches node_modules. This entry is redundant and can cause resolution/perf issues. Remove it, or, if you’re developing the package in-place, point to the repo root instead (e.g., path.resolve(__dirname, "..")).
}, - watchFolders: [ - path.join(__dirname, "node_modules", "react-native-sqlcipher-storage") - ] + // watchFolders: [path.resolve(__dirname, "..")] // only if developing the module from the repo root });example/android/build.gradle (1)
32-34: Use uri(File) for Maven repo path to handle Windows paths correctlyPassing a raw string can misbehave on Windows. Use uri(File).
- maven { - url = p.toRealPath().toString() - } + maven { + url = uri(p.toFile()) + }example-prev/App.tsx (1)
43-45: Avoid setState during unmount; close DB silently on unmountCalling closeDatabase() in componentWillUnmount triggers setState. Close directly without UI updates.
componentWillUnmount() { - this.closeDatabase(); + if (db) { + db.close().catch(() => {}); + db = undefined; + } }Also applies to: 180-186
package.json (4)
10-10: Use modern RN CLI for start script.local-cli is deprecated; use react-native start.
- "start": "node node_modules/react-native/local-cli/cli.js start", + "start": "react-native start",
49-62: Remove duplicate/incorrect ESLint package.Having both @eslint/js and a GitHub alias eslint-js is redundant; keep @eslint/js.
- "@eslint/js": "^9.37.0", + "@eslint/js": "^9.37.0", - "eslint-js": "github:eslint/js",
37-44: Tighten peerDependency ranges.Using "*" claims compatibility with all versions; set tested ranges to reduce breakage.
Example (adjust to your tested matrix):
- "peerDependencies": { - "electron": "*", - "react": "*", - "react-dom": "*", - "react-native": "*", - "react-native-web": "*", - "react-native-windows": "*" - }, + "peerDependencies": { + "electron": ">=22 <39", + "react": "^18.2.0", + "react-dom": "^18.2.0", + "react-native": ">=0.71 <0.83", + "react-native-web": ">=0.18 <0.22", + "react-native-windows": ">=0.71 <0.84" + },Please replace with the exact ranges you validate against.
74-76: Consider moving @journeyapps/sqlcipher to optionalDependencies.Prevents unnecessary native installs for RN-only consumers.
- "dependencies": { - "@journeyapps/sqlcipher": "^5.3.1" - }, + "dependencies": {}, + "optionalDependencies": { + "@journeyapps/sqlcipher": "^5.3.1" + },Ensure your Electron build path still installs/rebuilds this dependency (e.g., via electron-rebuild).
example/package.json (1)
11-11: Broaden lint target.Current glob misses nested files; lint the project recursively.
- "lint": "eslint *.{js,tsx}", + "lint": "eslint . --ext js,jsx,ts,tsx",react-native-sqlcipher-storage.d.ts (1)
6-13: Allow nulls in row values.SQLite cells can be null; widen value type.
- item(i: number): Record<string, string | number | boolean>; - _array: Record<string, string | number | boolean>[]; + item(i: number): Record<string, string | number | boolean | null>; + _array: Record<string, string | number | boolean | null>[];example/App.tsx (2)
47-47: Avoid module-level mutable state.Using a module-level
dbvariable can cause issues with React's hot reloading, multiple app instances, or test isolation. Consider usinguseRef(as suggested in the commented line 50) or a Context provider to manage database state.Apply this diff to use
useRefinstead:-let db: Database | null = null; - function App() { - // const db = useRef<Database | null>(null); + const db = useRef<Database | null>(null); const [state, dispatch] = useReducer(reducer, { progress: [] });Then update all references from
dbtodb.currentthroughout the component (lines 79, 98, 101, etc.).
164-458: Remove or document the commented code block.Nearly 300 lines of commented-out code should be removed to improve maintainability. If this code is needed for reference during migration, consider:
- Removing it entirely (it's available in git history).
- Moving it to a separate documentation file with migration notes.
- Adding a clear TODO comment explaining why it's temporarily retained.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (24)
example-prev/android/app/src/main/assets/www/2x.dbis excluded by!**/*.dbexample-prev/android/app/src/main/res/mipmap-hdpi/ic_launcher.pngis excluded by!**/*.pngexample-prev/android/app/src/main/res/mipmap-hdpi/ic_launcher_round.pngis excluded by!**/*.pngexample-prev/android/app/src/main/res/mipmap-mdpi/ic_launcher.pngis excluded by!**/*.pngexample-prev/android/app/src/main/res/mipmap-mdpi/ic_launcher_round.pngis excluded by!**/*.pngexample-prev/android/app/src/main/res/mipmap-xhdpi/ic_launcher.pngis excluded by!**/*.pngexample-prev/android/app/src/main/res/mipmap-xhdpi/ic_launcher_round.pngis excluded by!**/*.pngexample-prev/android/app/src/main/res/mipmap-xxhdpi/ic_launcher.pngis excluded by!**/*.pngexample-prev/android/app/src/main/res/mipmap-xxhdpi/ic_launcher_round.pngis excluded by!**/*.pngexample-prev/android/app/src/main/res/mipmap-xxxhdpi/ic_launcher.pngis excluded by!**/*.pngexample-prev/android/app/src/main/res/mipmap-xxxhdpi/ic_launcher_round.pngis excluded by!**/*.pngexample-prev/ios/Podfile.lockis excluded by!**/*.lockexample-prev/ios/example/www/2x.dbis excluded by!**/*.dbexample-prev/package-lock.jsonis excluded by!**/package-lock.jsonexample-prev/windows/example/Assets/LockScreenLogo.scale-200.pngis excluded by!**/*.pngexample-prev/windows/example/Assets/SplashScreen.scale-200.pngis excluded by!**/*.pngexample-prev/windows/example/Assets/Square150x150Logo.scale-200.pngis excluded by!**/*.pngexample-prev/windows/example/Assets/Square44x44Logo.scale-200.pngis excluded by!**/*.pngexample-prev/windows/example/Assets/Square44x44Logo.targetsize-24_altform-unplated.pngis excluded by!**/*.pngexample-prev/windows/example/Assets/StoreLogo.pngis excluded by!**/*.pngexample-prev/windows/example/Assets/Wide310x150Logo.scale-200.pngis excluded by!**/*.pngexample/ios/Podfile.lockis excluded by!**/*.lockexample/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (39)
.eslintignore(1 hunks).eslintrc.js(1 hunks)babel.config.js(1 hunks)electron/main.js(1 hunks)electron/renderer.js(1 hunks)example-prev/.gitignore(1 hunks)example-prev/.watchmanconfig(1 hunks)example-prev/App.tsx(1 hunks)example-prev/android/build.gradle(1 hunks)example-prev/android/gradle.properties(1 hunks)example-prev/android/settings.gradle(1 hunks)example-prev/babel.config.js(1 hunks)example-prev/ios/Podfile(1 hunks)example-prev/ios/example.xcworkspace/contents.xcworkspacedata(1 hunks)example-prev/metro.config.js(1 hunks)example-prev/package.json(1 hunks)example-prev/react-native.config.js(1 hunks)example-prev/tsconfig.json(1 hunks)example-prev/windows/.gitignore(1 hunks)example-prev/windows/example/Bundle/assets/app.json(1 hunks)example/.gitignore(1 hunks)example/.watchmanconfig(1 hunks)example/App.tsx(1 hunks)example/Gemfile(1 hunks)example/android/build.gradle(1 hunks)example/android/gradle.properties(1 hunks)example/android/settings.gradle(1 hunks)example/app.json(1 hunks)example/babel.config.js(1 hunks)example/index.js(1 hunks)example/ios/Podfile(1 hunks)example/ios/example.xcworkspace/contents.xcworkspacedata(1 hunks)example/metro.config.js(1 hunks)example/package.json(1 hunks)example/react-native.config.js(1 hunks)example/tsconfig.json(1 hunks)example/windows/.gitignore(1 hunks)package.json(2 hunks)react-native-sqlcipher-storage.d.ts(1 hunks)
✅ Files skipped from review due to trivial changes (7)
- example-prev/windows/example/Bundle/assets/app.json
- example-prev/ios/example.xcworkspace/contents.xcworkspacedata
- example-prev/tsconfig.json
- example-prev/windows/.gitignore
- .eslintignore
- example-prev/.gitignore
- example/.watchmanconfig
🚧 Files skipped from review as they are similar to previous changes (1)
- electron/renderer.js
🧰 Additional context used
🧬 Code graph analysis (3)
example/App.tsx (1)
ios/SQLite.h (1)
SQLite(33-35)
example-prev/App.tsx (1)
src/sqlite.core.js (1)
err(361-361)
electron/main.js (1)
ios/SQLite.h (1)
SQLite(33-35)
🪛 RuboCop (1.81.1)
example-prev/ios/Podfile
[convention] 54-55: Extra empty line detected at block body end.
(Layout/EmptyLinesAroundBlockBody)
example/Gemfile
[convention] 9-9: Gems should be sorted in an alphabetical order within their section of the Gemfile. Gem activesupport should appear before cocoapods.
(Bundler/OrderedGems)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (15)
example-prev/metro.config.js (1)
1-76: Critical inconsistency: Implementation does not match the described changes.The AI summary states this file "Replaces a large, manual Metro config with a call to makeMetroConfig from @rnx-kit/metro-config" and "Exports module as the result of makeMetroConfig({...})", but the actual implementation is a completely manual Metro configuration with no use of
makeMetroConfigor@rnx-kit/metro-configat all.Please clarify:
- Was the intent to use
@rnx-kit/metro-configbut the implementation was not completed?- Or is the AI summary incorrect and the manual configuration is intentional?
If
@rnx-kit/metro-configwas intended, based on the retrieved learnings, it's the recommended approach for React Native projects and would significantly simplify this configuration.example/ios/example.xcworkspace/contents.xcworkspacedata (1)
4-5: Verify setup instructions for the generated project path.The workspace now references a generated project at
../node_modules/.generated/ios/ReactTestApp.xcodeproj. Ensure that:
- Setup/installation instructions document the generation step required before opening this workspace
- The react-native-test-app setup command that generates this project is clearly documented
Without proper setup, developers attempting to open this workspace will encounter errors since the referenced project path won't exist initially.
Could you verify whether the README or setup documentation includes instructions for generating the ReactTestApp project? If not, consider adding a setup section or scripts to automate this step.
example/ios/Podfile (2)
7-7: Good: Explicit workspace naming improves clarity.Setting the workspace name explicitly aligns well with the workspace file changes and makes the configuration more transparent.
9-9: Verify: Confirm Fabric disabled is intentional for the example app.The configuration explicitly disables Fabric (
fabric_enabled => false). Since React Native's new architecture is the future direction, verify this choice is intentional for the example app.Is disabling Fabric intentional to maintain compatibility with older React Native versions, or should the example demonstrate the new architecture? Based on learnings about React Native 0.82 and the PR's dependency updates, consider whether enabling Fabric would be appropriate for demonstrating modern React Native capabilities.
example/tsconfig.json (1)
2-2: LGTM! Good use of the official React Native TypeScript config.Extending from
@react-native/typescript-config/tsconfig.jsonis the recommended approach for React Native projects. This simplifies maintenance and ensures consistency with React Native best practices.example/babel.config.js (1)
2-2: LGTM! Correct use of the modern React Native Babel preset.The update to
@react-native/babel-presetaligns with current React Native tooling and best practices.example-prev/android/gradle.properties (1)
20-21: LGTM! Standard AndroidX configuration.Enabling AndroidX and Jetifier is the correct approach for modern Android projects and aligns with current React Native Android setup requirements.
.eslintrc.js (1)
1-4: LGTM! Standard React Native ESLint configuration with Prettier.The configuration correctly integrates React Native linting rules with Prettier formatting, and
root: trueappropriately prevents ESLint from searching parent directories for additional configs.babel.config.js (1)
1-3: All Babel configs consistently use themodule:prefix; no change needed.Likely an incorrect or invalid review comment.
example-prev/android/settings.gradle (1)
1-6: LGTMSettings wiring looks correct for the example project and module include.
example/app.json (1)
2-16: LGTMManifest fields and resource paths align with React Native Test App conventions.
example/android/gradle.properties (1)
29-31: android.jetifier.ignorelist is a valid Jetifier property Configuration in gradle.properties is correct.example/index.js (1)
1-9: LGTM!Standard React Native entry file with correct imports and component registration.
example/App.tsx (2)
140-144: Verify async cleanup pattern.The
closeDatabaseasync function is called in the cleanup without being awaited. While this is a common pattern for database cleanup, ensure that any errors during unmount are handled appropriately and won't cause issues if the component unmounts before the close completes.
461-528: LGTM!StyleSheet definitions are correct and the new
liErrorTextstyle properly supports error rendering in the progress list.
| buildscript { | ||
| ext { | ||
| buildToolsVersion = "28.0.3" | ||
| minSdkVersion = 16 | ||
| compileSdkVersion = 28 | ||
| targetSdkVersion = 28 | ||
| } | ||
| repositories { | ||
| google() | ||
| jcenter() | ||
| } | ||
| dependencies { | ||
| classpath("com.android.tools.build:gradle:3.4.2") | ||
|
|
||
| // NOTE: Do not place your application dependencies here; they belong | ||
| // in the individual module build.gradle files | ||
| } |
There was a problem hiding this comment.
Severely outdated Android tooling versions.
The Android Gradle configuration uses very old versions from 2019:
- Android Gradle Plugin 3.4.2 (current is 8.x)
- Build tools 28.0.3 (current is 34.x)
- compileSdk/targetSdk 28 (Android 9; current is 34/Android 14)
This creates multiple issues:
- Google Play Store requires targetSdk 33+ for app submissions (since Aug 2023)
- Missing critical security patches
- Incompatible with modern Android libraries
- Cannot use newer Android features
If this example is intended to be functional, update to current versions:
ext {
- buildToolsVersion = "28.0.3"
+ buildToolsVersion = "34.0.0"
minSdkVersion = 16
- compileSdkVersion = 28
- targetSdkVersion = 28
+ compileSdkVersion = 34
+ targetSdkVersion = 34
} dependencies {
- classpath("com.android.tools.build:gradle:3.4.2")
+ classpath("com.android.tools.build:gradle:8.5.0")Alternatively, if this is a legacy/archived example, consider removing it from the PR or clearly marking it as unsupported.
📝 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.
| buildscript { | |
| ext { | |
| buildToolsVersion = "28.0.3" | |
| minSdkVersion = 16 | |
| compileSdkVersion = 28 | |
| targetSdkVersion = 28 | |
| } | |
| repositories { | |
| google() | |
| jcenter() | |
| } | |
| dependencies { | |
| classpath("com.android.tools.build:gradle:3.4.2") | |
| // NOTE: Do not place your application dependencies here; they belong | |
| // in the individual module build.gradle files | |
| } | |
| buildscript { | |
| ext { | |
| buildToolsVersion = "34.0.0" | |
| minSdkVersion = 16 | |
| compileSdkVersion = 34 | |
| targetSdkVersion = 34 | |
| } | |
| repositories { | |
| google() | |
| jcenter() | |
| } | |
| dependencies { | |
| classpath("com.android.tools.build:gradle:8.5.0") | |
| // NOTE: Do not place your application dependencies here; they belong | |
| // in the individual module build.gradle files | |
| } | |
| } |
🤖 Prompt for AI Agents
In example-prev/android/build.gradle around lines 3 to 19 the Android tooling is
severely outdated; update the Android Gradle Plugin to an 8.x release (e.g.,
com.android.tools.build:gradle:8.1.0) and ensure the Gradle wrapper uses a
matching Gradle 8.x distribution, set compileSdkVersion and targetSdkVersion to
34 (or at least 33+ to meet Play Store requirements), update buildToolsVersion
to 34.x (or remove it since modern AGP manages build tools), and consider
raising minSdkVersion to a sensible level (e.g., 21) as needed; also ensure
repositories include google() and mavenCentral(); alternatively, if this module
is intentionally legacy, mark/remove it from the PR as unsupported.
| closeDb = async () => { | ||
| try { | ||
| await db.close(); | ||
| this.addProgress("Database CLOSED"); | ||
| } | ||
| catch(error) { | ||
| this.errorCB(error.message); | ||
| } | ||
| } |
There was a problem hiding this comment.
Guard closeDb when db is undefined; clear handle after closing
Prevents errors when runBadPwd calls closeDb with no open DB and avoids stale handle.
- closeDb = async () => {
- try {
- await db.close();
- this.addProgress("Database CLOSED");
- }
- catch(error) {
- this.errorCB(error.message);
- }
- }
+ closeDb = async () => {
+ if (!db) return;
+ try {
+ await db.close();
+ db = undefined;
+ this.addProgress("Database CLOSED");
+ } catch (error) {
+ this.errorCB(error.message);
+ }
+ }📝 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.
| closeDb = async () => { | |
| try { | |
| await db.close(); | |
| this.addProgress("Database CLOSED"); | |
| } | |
| catch(error) { | |
| this.errorCB(error.message); | |
| } | |
| } | |
| closeDb = async () => { | |
| if (!db) return; | |
| try { | |
| await db.close(); | |
| db = undefined; | |
| this.addProgress("Database CLOSED"); | |
| } catch (error) { | |
| this.errorCB(error.message); | |
| } | |
| } |
🤖 Prompt for AI Agents
In example-prev/App.tsx around lines 170-178, guard closeDb so it first checks
whether the db handle exists before attempting to close it and returns early if
not, then after a successful close clear the stored handle (set the db
property/variable to null or undefined) to avoid a stale reference; keep the
existing try/catch flow and only call addProgress("Database CLOSED") when a
close actually occurred, and still call errorCB(error.message) on failure.
| module.exports = { | ||
| presets: ['module:metro-react-native-babel-preset'], | ||
| }; |
There was a problem hiding this comment.
Inconsistent Babel preset with other configs in this PR.
This file uses the older metro-react-native-babel-preset, while example/babel.config.js (line 2) and the root babel.config.js (line 2) use the modern @react-native/babel-preset. For consistency and to align with current React Native tooling, consider updating this preset.
Apply this diff to align with the other Babel configs:
module.exports = {
- presets: ['module:metro-react-native-babel-preset'],
+ presets: ['module:@react-native/babel-preset'],
};📝 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.
| module.exports = { | |
| presets: ['module:metro-react-native-babel-preset'], | |
| }; | |
| module.exports = { | |
| presets: ['module:@react-native/babel-preset'], | |
| }; |
🤖 Prompt for AI Agents
In example-prev/babel.config.js around lines 1 to 3, the preset is set to
'module:metro-react-native-babel-preset' which is inconsistent with other
configs; update the preset to '@react-native/babel-preset' to match
example/babel.config.js and the root babel.config.js, ensuring the exported
object remains the same (module.exports = { presets:
['@react-native/babel-preset'] };).
| @@ -0,0 +1,55 @@ | |||
| platform :ios, '9.0' | |||
There was a problem hiding this comment.
iOS 9.0 deployment target is critically outdated.
iOS 9.0 (released 2015) is incompatible with modern React Native versions. Current React Native stable (0.82) and the versions mentioned in the PR context typically require iOS 12.0 or higher as the minimum deployment target. Building with iOS 9.0 will likely result in compilation errors and pod installation failures.
Apply this diff to update to a modern iOS deployment target:
-platform :ios, '9.0'
+platform :ios, '12.4'📝 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.
| platform :ios, '9.0' | |
| platform :ios, '12.4' |
🤖 Prompt for AI Agents
In example-prev/ios/Podfile around line 1, the deployment target is set to iOS
9.0 which is too old for modern React Native; update the platform line to a
current minimum (e.g. platform :ios, '12.0' or higher) and then run pod install;
also ensure the Xcode project deployment target matches this new minimum (update
project settings or use a post_install hook to set the iOS deployment target
across pods if needed).
| target 'example-tvOS' do | ||
| # Pods for example-tvOS | ||
|
|
||
| target 'example-tvOSTests' do | ||
| inherit! :search_paths | ||
| # Pods for testing | ||
| end | ||
|
|
||
| end |
There was a problem hiding this comment.
tvOS target missing use_native_modules! and has formatting issue.
The example-tvOS target doesn't call use_native_modules!, which means native modules (including the sqlcipher storage module) won't be automatically linked for tvOS builds. Additionally, there's an extra empty line at the end of the block.
Apply this diff to add native modules support and fix formatting:
target 'example-tvOS' do
# Pods for example-tvOS
target 'example-tvOSTests' do
inherit! :search_paths
# Pods for testing
end
-
+
+ use_native_modules!
end📝 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.
| target 'example-tvOS' do | |
| # Pods for example-tvOS | |
| target 'example-tvOSTests' do | |
| inherit! :search_paths | |
| # Pods for testing | |
| end | |
| end | |
| target 'example-tvOS' do | |
| # Pods for example-tvOS | |
| target 'example-tvOSTests' do | |
| inherit! :search_paths | |
| # Pods for testing | |
| end | |
| use_native_modules! | |
| end |
🧰 Tools
🪛 RuboCop (1.81.1)
[convention] 54-55: Extra empty line detected at block body end.
(Layout/EmptyLinesAroundBlockBody)
🤖 Prompt for AI Agents
In example-prev/ios/Podfile around lines 47 to 55, the example-tvOS target is
missing a call to use_native_modules! and has an extra blank line at the end of
the block; modify the example-tvOS target to call use_native_modules! (placed
inside the target, typically after the comment and before the nested tests
target) and remove the trailing empty line so the block is consistently indented
and formatted.
example/App.tsx
Outdated
| const populateDatabase = useCallback(async () => { | ||
| addProgress("Populating database"); | ||
| return Promise.resolve(); | ||
| }, [addProgress]); |
There was a problem hiding this comment.
Critical: Database population logic is missing.
The populateDatabase function is a stub that doesn't create tables or insert data. The old implementation (lines 186-326 in the commented code) contained the actual database schema creation and data insertion logic. Without this, the demo won't work.
Do you want me to help restore the population logic from the commented implementation, or is this intentionally left as a TODO?
🤖 Prompt for AI Agents
In example/App.tsx around lines 64 to 67, the populateDatabase function is
currently a stub and does not create tables or insert seed data; restore the
previous implementation from the commented block (lines 186-326) so the demo
works: implement creating the DB schema (tables and indexes), insert initial
demo data inside a transaction, call addProgress at key steps, handle and log
errors and roll back on failure, and return a resolved Promise when complete;
ensure async/await usage and proper cleanup (closing connections) match the
original logic.
example/App.tsx
Outdated
| const runBadPwd = useCallback(() => {}, []); | ||
| const migrateDb = useCallback(() => {}, []); |
There was a problem hiding this comment.
Implement or remove stub callbacks.
Both runBadPwd and migrateDb are empty stubs but their corresponding buttons are still rendered (lines 152-153). This creates a broken user experience. Either implement the functionality (old implementations available in lines 402-437 of commented code) or remove the buttons.
🤖 Prompt for AI Agents
In example/App.tsx around lines 121-122, the callbacks runBadPwd and migrateDb
are empty stubs while their buttons are still rendered (buttons at ~lines
152-153), causing a broken UX; restore the actual implementations from the
commented old code block at lines 402-437 into these useCallback definitions
(preserve async behavior, error handling, and any state updates) and add correct
dependency arrays/types, or if you opt to remove them, delete the buttons and
any related state/props/hooks referenced only by those actions; ensure UI and
tests reflect the chosen change.
| # Cocoapods 1.15 introduced a bug which break the build. We will remove the upper | ||
| # bound in the template on Cocoapods with next React Native release. | ||
| gem 'cocoapods', '>= 1.13', '< 1.15' |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the CocoaPods 1.15 bug scope; consider relaxing the upper bound.
If the issue was limited to a specific 1.15.x, prefer excluding only the bad versions (e.g., != 1.15.0) or allowing a patched 1.15.x/1.16+.
🌐 Web query:
What specific build-breaking bug was introduced in CocoaPods 1.15 for React Native projects, and in which patch/minor version was it fixed? Is it safe to allow CocoaPods >= 1.15.x (patched) or newer?
💡 Result:
Short answer: CocoaPods 1.15.0 contained regressions in the pod install/project-generator behavior (notably a change to FileUtils.cp_r usage that changed how pod folders were copied/overwritten and other install-related changes) which caused React Native iOS builds to break. Those regressions were reverted/fixed in the 1.15.x patch series — the fixes were published in 1.15.2. It is safe to allow CocoaPods >= 1.15.x provided you require the patched versions (>= 1.15.2).
Sources: CocoaPods releases (1.15.0, 1.15.1, 1.15.2) and the PR reverting the FileUtils change. [1][2]
References
- CocoaPods releases notes (1.15.0 / 1.15.1 / 1.15.2) — shows regressions in 1.15.0 and that 1.15.2 reverts the offending changes. [1]
- PR: "Don't use :remove_destination option to FileUtils.cp_r" (explains the copy/remove change that caused unexpected install layout). [2]
Relax CocoaPods upper bound to allow patched 1.15.x
In example/Gemfile (lines 6–8), update the constraint to:
gem 'cocoapods', '>= 1.13', '< 2.0', '!= 1.15.0', '!= 1.15.1'Excludes only the broken 1.15.0/1 versions, allows 1.15.2+ and future releases.
🤖 Prompt for AI Agents
In example/Gemfile around lines 6 to 8, the current CocoaPods version constraint
blocks all 1.15.x releases; update the Gemfile to relax the upper bound so it
excludes only the broken 1.15.0 and 1.15.1 versions while allowing 1.15.2+ and
future major releases by changing the gem constraint to allow >= 1.13, < 2.0 and
explicitly exclude 1.15.0 and 1.15.1.
| ws_dir = Pathname.new(__dir__) | ||
| ws_dir = ws_dir.parent until | ||
| File.exist?("#{ws_dir}/node_modules/react-native-test-app/test_app.rb") || | ||
| ws_dir.expand_path.to_s == '/' | ||
| require "#{ws_dir}/node_modules/react-native-test-app/test_app.rb" |
There was a problem hiding this comment.
Critical: Add error handling for missing react-native-test-app.
The directory traversal logic finds the workspace root but doesn't verify that test_app.rb was actually found before requiring it on line 5. If react-native-test-app is not installed or the loop exits at the filesystem root, line 5 will fail with a "cannot load such file" error.
Apply this diff to add proper error handling:
ws_dir = Pathname.new(__dir__)
ws_dir = ws_dir.parent until
File.exist?("#{ws_dir}/node_modules/react-native-test-app/test_app.rb") ||
ws_dir.expand_path.to_s == '/'
+
+test_app_path = "#{ws_dir}/node_modules/react-native-test-app/test_app.rb"
+unless File.exist?(test_app_path)
+ raise "react-native-test-app not found. Please run 'npm install' or 'yarn install' first."
+end
+
-require "#{ws_dir}/node_modules/react-native-test-app/test_app.rb"
+require test_app_path🤖 Prompt for AI Agents
In example/ios/Podfile around lines 1 to 5, the traversal may reach filesystem
root without finding node_modules/react-native-test-app/test_app.rb but the file
is required unconditionally on line 5; after the loop verify that the target
file exists and if not either raise a clear error or abort with a descriptive
message (including guidance to install react-native-test-app), otherwise require
the found file path; ensure the require uses the verified path to avoid a
"cannot load such file" runtime crash.
|
|
||
| *.binlog | ||
| *.hprof | ||
| *.sln |
There was a problem hiding this comment.
Do not ignore the solution file.
The Windows sample needs its .sln checked in; ignoring *.sln prevents adding or updating the solution, breaking the build/setup of the example project.
Please drop this pattern:
-*.sln📝 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.
| *.sln |
🤖 Prompt for AI Agents
In example/windows/.gitignore around line 28, the pattern "*.sln" is incorrectly
ignoring the Visual Studio solution file required for the Windows sample; remove
the "*.sln" entry from the .gitignore so the .sln file can be tracked, then
commit the .sln to the repo (or if you need to ignore only generated solution
files, replace the broad pattern with a more specific exclude that does not
match the project solution).
| const openPath = path.resolve(path.join(this.#dbLocation, options.name)); | ||
| const db = await AsyncDatabase.newDatabase(openPath); | ||
| if (options.key) { | ||
| await db.run(`PRAGMA key = '${options.key}'`); |
There was a problem hiding this comment.
Bug: Encryption Key Opens Door to SQL Injection
Database encryption key is directly interpolated into SQL string using template literals, creating a SQL injection vulnerability. If the key contains quotes or SQL syntax, it could break out and inject arbitrary commands. The key should be passed through a parameterized approach or properly escaped.
| rowsAffected: all.changes, | ||
| insertId: all.insertId | ||
| }; | ||
| } |
There was a problem hiding this comment.
Bug: Type Contract Broken: Non-SELECT Queries Lack Expected Data
Non-SELECT query results are missing the rows property, but the return type annotation (line 135) declares rows as a required property. This causes type mismatch where consumers expect rows to always be present, but it's undefined for INSERT/UPDATE/DELETE operations.
| const openPath = path.resolve(path.join(this.#dbLocation, options.name)); | ||
| const db = await AsyncDatabase.newDatabase(openPath); | ||
| if (options.key) { | ||
| await db.run(`PRAGMA key = '${options.key}'`); |
There was a problem hiding this comment.
Bug: SQL injection vulnerability in PRAGMA key
The database encryption key is directly interpolated into the SQL string using template literals without parameterization. An attacker controlling options.key could inject arbitrary SQL code, compromising database security and application integrity.
| const all = await statement.run(params); | ||
| result = { | ||
| rowsAffected: all.changes, | ||
| insertId: all.insertId, |
There was a problem hiding this comment.
Bug: Wrong property name causes undefined insertId for non-SELECT queries
The AsyncStatement.run() method resolves with an object containing lastID and changes properties, but line 153 accesses all.insertId which doesn't exist. This causes insertId to always be undefined for INSERT/UPDATE/DELETE operations. The property should be all.lastID to match the sqlite3 API, consistent with how SELECT queries handle it on line 146.
|
|
||
| let result; | ||
|
|
||
| if (sql.toLocaleLowerCase().startsWith("select")) { |
There was a problem hiding this comment.
Bug: SELECT detection fails for queries with leading whitespace
The startsWith("select") check fails to identify SELECT queries that have leading whitespace, comments, or use CTEs (e.g., WITH ... SELECT). Such queries would be incorrectly processed using statement.run() instead of statement.all(), causing them to return no rows. A trimmed or regex-based approach would handle these valid SQL patterns correctly.
| const openPath = path.resolve(path.join(this.#dbLocation, options.name)); | ||
| const db = await AsyncDatabase.newDatabase(openPath); | ||
| if (options.key) { | ||
| await db.run(`PRAGMA key = '${options.key}'`); |
There was a problem hiding this comment.
Bug: Encryption keys containing single quotes break database opening
The encryption key is directly interpolated into the PRAGMA statement without escaping single quotes. If options.key contains a single quote character (e.g., a passphrase like it's secret), the resulting SQL becomes malformed and the database will fail to open with a syntax error.
| const all = await statement.run(params); | ||
| result = { | ||
| rowsAffected: all.changes, | ||
| insertId: all.insertId, |
There was a problem hiding this comment.
Bug: Wrong property name causes undefined insertId for writes
The #all method accesses all.insertId for non-SELECT queries, but AsyncStatement.run returns an object with lastID property (not insertId). This is inconsistent with the SELECT branch at line 146 which correctly uses all.lastID. As a result, insertId will always be undefined for INSERT/UPDATE/DELETE operations, breaking insert ID tracking functionality.
| const openPath = path.resolve(path.join(this.#dbLocation, options.name)); | ||
| const db = await AsyncDatabase.newDatabase(openPath); | ||
| if (options.key) { | ||
| await db.run(`PRAGMA key = '${options.key}'`); |
There was a problem hiding this comment.
Bug: SQL injection vulnerability in database key parameter
The encryption key is interpolated directly into the SQL string using template literals without sanitization. If options.key contains a single quote, it could break the SQL or allow injection. Since this value comes from IPC (renderer process), a malicious or compromised renderer could exploit this to execute arbitrary SQL commands.
| result = { | ||
| rowsAffected: all.changes, | ||
| insertId: all.insertId, | ||
| }; |
There was a problem hiding this comment.
Bug: Missing rows property for non-SELECT query results
For non-SELECT queries, the #all method returns an object without a rows property. When destructured at line 97, rows becomes undefined and is assigned to the result object at line 110. Consumers expecting rows to always be an array will encounter undefined, which could cause runtime errors when iterating over results.
| const all = await statement.run(params); | ||
| result = { | ||
| rowsAffected: all.changes, | ||
| insertId: all.insertId, |
There was a problem hiding this comment.
Bug: Wrong property name for insert ID in run result
The #all method accesses all.insertId for non-SELECT queries, but the AsyncStatement.run() method resolves with the sqlite3 context object which exposes the property as lastID, not insertId. The SELECT branch correctly uses all.lastID (line 146), but the non-SELECT branch uses all.insertId, which will always be undefined. This causes insert ID tracking to fail for INSERT/UPDATE/DELETE operations.
| result = { | ||
| rowsAffected: all.changes, | ||
| insertId: all.insertId, | ||
| }; |
There was a problem hiding this comment.
Bug: Non-SELECT queries return undefined rows instead of array
The #all method returns a result object without a rows property for non-SELECT queries (INSERT/UPDATE/DELETE), while SELECT queries include rows. When destructured in backgroundExecuteSqlBatch, this causes rows to be undefined for non-SELECT operations. Client code expecting rows to always be an array will fail with TypeError when accessing properties like length or iterating over the result.
| const all = await statement.run(params); | ||
| result = { | ||
| rowsAffected: all.changes, | ||
| insertId: all.insertId, |
There was a problem hiding this comment.
Bug: Wrong property name causes undefined insertId
The non-SELECT branch of #all accesses all.insertId, but AsyncStatement.run() resolves with the SQLite statement context which has lastID property, not insertId. The SELECT branch correctly uses all.lastID on line 146. This causes insertId to always be undefined for INSERT, UPDATE, and DELETE operations, breaking the ability to get the last inserted row ID.
| const openPath = path.resolve(path.join(this.#dbLocation, options.name)); | ||
| const db = await AsyncDatabase.newDatabase(openPath); | ||
| if (options.key) { | ||
| await db.run(`PRAGMA key = '${options.key}'`); |
There was a problem hiding this comment.
Bug: SQL injection via unsanitized key in PRAGMA
The options.key value is directly interpolated into the SQL string for PRAGMA key without any sanitization or escaping. Since this value comes from the renderer process via IPC and could contain single quotes or other SQL-significant characters, a malicious or malformed key value could break the SQL statement or potentially allow SQL injection attacks.
| const all = await statement.run(params); | ||
| result = { | ||
| rowsAffected: all.changes, | ||
| insertId: all.insertId, |
There was a problem hiding this comment.
Bug: Wrong property name causes undefined insertId for writes
In the #all method, when handling non-SELECT queries (INSERT, UPDATE, DELETE), the code accesses all.insertId but the AsyncStatement.run method returns the sqlite3 statement context which has a lastID property, not insertId. The SELECT branch at line 146 correctly uses all.lastID. This causes insertId to always be undefined for INSERT statements, breaking functionality that depends on retrieving the last inserted row ID.
| const all = await statement.run(params); | ||
| result = { | ||
| rowsAffected: all.changes, | ||
| insertId: all.insertId, |
There was a problem hiding this comment.
Bug: Wrong property name causes undefined insertId for writes
The code accesses all.insertId but the sqlite3 callback context returned by AsyncStatement.run() provides lastID, not insertId. This causes insertId to always be undefined for INSERT/UPDATE/DELETE operations. The SELECT branch on line 146 correctly uses all.lastID, but this non-SELECT branch uses the non-existent all.insertId property.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 5 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| const all = await statement.run(params); | ||
| result = { | ||
| rowsAffected: all.changes, | ||
| insertId: all.insertId, |
There was a problem hiding this comment.
Wrong property name causes undefined insertId for writes
High Severity
The #all method accesses all.insertId for non-SELECT queries, but AsyncStatement.run() returns the sqlite3 callback context which has a lastID property, not insertId. This causes insertId to always be undefined for INSERT, UPDATE, and DELETE operations. The SELECT branch correctly uses all.lastID at line 146, but the non-SELECT branch at line 153 incorrectly uses all.insertId.
| const openPath = path.resolve(path.join(this.#dbLocation, options.name)); | ||
| const db = await AsyncDatabase.newDatabase(openPath); | ||
| if (options.key) { | ||
| await db.run(`PRAGMA key = '${options.key}'`); |
There was a problem hiding this comment.
SQL injection vulnerability in PRAGMA key statement
Medium Severity
The encryption key is directly interpolated into the SQL string using template literals: `PRAGMA key = '${options.key}'`. If the key contains single quotes or other special characters, it could break the query or potentially allow SQL injection. The key value comes from IPC which could be influenced by renderer process code.
| rowsAffected: all.changes, | ||
| insertId: all.insertId, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Missing rows property for non-SELECT query results
Medium Severity
The #all method's non-SELECT branch (lines 150-154) omits the rows property from the result object. When backgroundExecuteSqlBatch destructures rows from this result at line 97, it receives undefined instead of an empty array. This undefined value propagates to the response at line 110, violating the documented return type (rows: any[]) and potentially causing runtime errors if consumers attempt array operations like rows.length or rows.forEach().
| await db.run("PRAGMA cipher_migrate"); | ||
| } | ||
| this.#databases.set(options.name, db); | ||
| }; |
There was a problem hiding this comment.
Database connection leaked when PRAGMA commands fail
Medium Severity
In the open method, if the PRAGMA key or PRAGMA cipher_migrate commands fail after the database connection is created at line 35, the connection is never closed and never stored in #databases. This creates an orphaned SQLite connection that holds file locks and memory, with no way to clean it up. Repeated failed open attempts (e.g., with malformed keys) will accumulate leaked connections.
| await statement.finalize(); | ||
|
|
||
| return result; | ||
| } |
There was a problem hiding this comment.
Prepared statement leaked when query execution fails
Medium Severity
In the #all method, if statement.all(params) or statement.run(params) throws an error (e.g., constraint violation, type mismatch, or invalid parameters), statement.finalize() at line 157 is never called. The prepared statement remains allocated, leaking resources. In a long-running Electron app with frequent query failures, this accumulates unfinalizable statements.
Summary by CodeRabbit
New Features
Refactor
Chores
Revert/Removal
Note
Adds desktop support via Electron with an encrypted SQLite (SQLCipher) bridge and a runnable example app.
electron/main.jsimplements SQLCipher access with async wrappers, statement helpers, and IPC handlers forsqlite:open,sqlite:close,sqlite:delete, andsqlite:backgroundExecuteSqlBatchelectron/preload.jsexposeswindow.sqliteapi(open/close/delete/batch) viacontextBridge;electron/renderer.jsprovides a thin renderer wrapper; d.ts files includedexample/electron(Forge + Webpack): main/preload/renderer TS entries, webpack configs, TS config, and package.json wiring to librarybabel.config.jsadded;.gitignoreupdated; new test app config underexamplefor bundling assetsexample-prevfor referenceWritten by Cursor Bugbot for commit fdb912e. This will update automatically on new commits. Configure here.