Skip to content

chore(deps): Update NitroSQLite to 9.8.1 #101437

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chrispader wants to merge 6 commits into
base: main
Choose a base branch
from
Open

chore(deps): Update NitroSQLite to 9.8.1 #101437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
89b4221
chore(deps): prepare NitroSQLite 9.8.0 from upstream commit
chrispader Sep 17, 2026
719b37c
fix(ios): configure NitroSQLite storage in host plists
chrispader Sep 17, 2026
32eea6c
chore(deps): use published NitroSQLite 9.8.0
chrispader Sep 17, 2026
4ad7d28
chore(deps): isolate NitroSQLite 9.8.1 upgrade
chrispader Sep 17, 2026
6151782
chore(ios): keep unrelated native lockfile state
chrispader Sep 17, 2026
71a4151
chore(deps): merge main into NitroSQLite upgrade
chrispader Sep 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ios/NewExpensify/Info.plist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@
</array>
<key>RCTNewArchEnabled</key>
<true/>
<key>RNNitroSQLite_DatabaseLocation</key>
<string>ApplicationSupport</string>
Comment on lines +98 to +99

@ikevin127 ikevin127 Sep 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 ios/NewExpensify/Info.plist:98

Correct. Key and value match 9.8.1's ios/OnLoad.mm, and the companion PR sets the same pair. Neither plist sets RNNitroSQLite_AppGroup, which matters because the app-group branch returns before RNNitroSQLite_DatabaseLocation is ever read.

Worth a comment in the plist or a note on the issue so nobody adds an App Group later and quietly reverts the Files-app fix.

<key>UIAppFonts</key>
<array>
<string>ExpensifyNewKansas-Medium.otf</string>
Expand Down
4 changes: 2 additions & 2 deletions ios/Podfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3935,7 +3935,7 @@ PODS:
- SocketRocket
- Turf
- Yoga
- RNNitroSQLite (9.6.0):
- RNNitroSQLite (9.8.1):
- boost
- DoubleConversion
- fast_float
Expand Down Expand Up @@ -5118,7 +5118,7 @@ SPEC CHECKSUMS:
RNLiveMarkdown: 318b3defc20b75a21ca00d58342109230c044da5
RNLocalize: 05e367a873223683f0e268d0af9a8a8e6aed3b26
rnmapbox-maps: 8b7629ef3ae59dd96340470e568cbc7c08c54ff9
RNNitroSQLite: a9b5965d511ed6e99ce903380e64934d043a0d2c
RNNitroSQLite: 06ff2497ab8b6c75b780f080f23c99555e7bf4a5
RNPermissions: 518f0a0c439acc74e2b9937e0e7d29e5031ae949
RNReactNativeHapticFeedback: 5f1542065f0b24c9252bd8cf3e83bc9c548182e4
RNReanimated: de3eaf0bd2aec5036554a125eb02c28b59de45ae
Expand Down
8 changes: 4 additions & 4 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@
"react-native-localize": "^3.5.4",
"react-native-nitro-fetch": "1.5.4",
"react-native-nitro-modules": "0.36.3",
"react-native-nitro-sqlite": "9.6.0",
"react-native-nitro-sqlite": "9.8.1",

@ikevin127 ikevin127 Sep 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 package.json:203 (behaviour lands in src/libs/ExportOnyxState/index.native.ts:19)

Settings → Troubleshoot → Export Onyx state breaks silently on iOS and Android.

9.8.1's cpp/operations.cpp is byte-identical to 9.8.0:

void sqliteOpenDb(const std::string& dbName, const std::string& docPath) {
  std::lock_guard lifecycleLock(dbLifecycleMutex);
  {
    std::lock_guard lock(dbMapMutex);
    if (dbMap.contains(dbName)) {
      throw NitroSQLiteException::DatabaseAlreadyOpen(dbName);
    }
  }

and the JS layer still rejects it before the native call is even reached:

// 9.8.1 lib/module/operations/session.js
export function open(options) {
  openDatabaseQueue(options.name); // throws "Database OnyxDB is already open."

We open OnyxDB a second time here, while Onyx's SQLiteProvider already holds it:

// src/libs/ExportOnyxState/index.native.ts
onyxDb = open({name: CONST.DEFAULT_DB_NAME});

9.6.0 tolerated this because sqliteOpenDb just did dbMap[dbName] = db;. The throw happens inside the new Promise executor in readFromOnyxDatabase, and TroubleshootPage.tsx:92 has no .catch, so the user taps Export Onyx state and gets nothing at all: no share sheet, no error, no log. That is our primary tool for debugging user reports.

This is the exact blocker the deleted patches/react-native-nitro-sqlite/details.md recorded:

its new per-database queue breaks second opens of the same database (used by src/libs/ExportOnyxState/index.native.ts)

The SQLITE_THREADSAFE=0 half of that note is genuinely fixed (9.8.1's podspec still defaults threadSafe to true). The second-open half is not, and the note documenting it is being deleted.

Fix belongs in src/libs/ExportOnyxState/index.native.ts: reuse Onyx's existing connection instead of opening a new one, or read through Onyx's own API. At minimum add a .catch in TroubleshootPage.tsx so it fails loudly.

Please also add "Troubleshoot → Export Onyx state, verify the share sheet opens with a populated dump" to the Tests section and run it on both platforms.

@ikevin127 ikevin127 Sep 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 package.json:203

9.8.1 still routes every async op through a per-database JS queue, one at a time with a setImmediate hop between each:

return queueOperationAsync(dbName, () => executeAsyncNative(dbName, query, params));

Onyx fans out and expects overlap:

return Promise.all(keyChunks.map((keyChunk) => provider.store.executeAsync(command, keyChunk)))

On a High Traffic account that is many chunks now running strictly sequentially instead of concurrently on the native pool. Please post a TTI comparison against main with a High Traffic account on a real low-end device, both platforms, before this merges.

"Send a message and relaunch" will not surface it.

@ikevin127 ikevin127 Sep 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 package.json:203 (new, from the 9.8.1 respin)

9.8.1's shipped nitrogen/generated/ C++ was generated by nitrogen 0.37.1 (devDependencies.nitrogen: "0.37.1"), while this PR keeps react-native-nitro-modules pinned at 0.36.3.

The peer range that would have caught a mismatch was hand-relaxed from >=0.37.1 to >=0.35.0 specifically to allow this, so it is now a declaration rather than a check.

I did verify every NitroModules header the generated and hand-written C++ includes exists in the installed 0.36.3:

AnyMapUtils.hpp, ArrayBuffer.hpp, ArrayBufferHolder.hpp, DateToChronoDate.hpp,
DefaultConstructableObject.hpp, HybridObject.hpp, HybridObjectRegistry.hpp,
JHybridObject.hpp, JSIConverter.hpp, JSIHelpers.hpp, NitroDefines.hpp,
Null.hpp, Promise.hpp, PropNameIDCache.hpp, RuntimeError.hpp

none missing, so it looks buildable. That is not proof though, signatures inside those headers can still have moved between 0.36.3 and 0.37.1. The only thing that settles it is an actual native build.

The HybridApp bot already asked for one on this PR, so please run AdHoc builds for both iOS and Android (standalone and hybrid) and link them here.

@ikevin127 ikevin127 Sep 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 package.json:203

9.8.1's podspec still picks build defaults when we say nothing:

app_config = app_package.fetch("nitroSQLite", {})
thread_safe_value = app_config.fetch("threadSafe", true)
performance_mode = app_config.fetch("performanceMode", true)

performanceMode defaulting to true newly enables -DSQLITE_DQS=0 -DSQLITE_DEFAULT_MEMSTATUS=0 -DSQLITE_DEFAULT_WAL_SYNCHRONOUS=1 -DSQLITE_OMIT_SHARED_CACHE=1 ... on the iOS pod, and Android gets none of it (it reads rootProject.properties['nitroSqliteFlags'], which we do not set).

Two platforms compiling SQLite differently, off defaults we never chose. Suggest pinning them so the build is reproducible and the choice is on the record:

"nitroSQLite": {
    "threadSafe": true,
    "performanceMode": true
}

"react-native-onyx": "3.0.111",
"react-native-pager-view": "9.0.4",
"react-native-pdf": "7.0.2",
Expand Down
35 changes: 0 additions & 35 deletions patches/react-native-nitro-sqlite/details.md
View file
Open in desktop

This file was deleted.

30 changes: 0 additions & 30 deletions ...te/react-native-nitro-sqlite+9.6.0+001+dont-mask-original-error-on-rollback-failure.patch
View file
Open in desktop

This file was deleted.

95 changes: 0 additions & 95 deletions ...e-nitro-sqlite/react-native-nitro-sqlite+9.6.0+002+store-database-outside-documents.patch
View file
Open in desktop

This file was deleted.

16 changes: 5 additions & 11 deletions tests/unit/MoveFilesOutOfDocumentsTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,18 +165,12 @@ describe('MoveFilesOutOfDocuments migration (native)', () => {
await expect(MoveFilesOutOfDocuments()).resolves.toBeUndefined();
});

it('keeps the native database migration patch in sync with the database name', () => {
const patchDir = path.resolve(__dirname, '../../patches/react-native-nitro-sqlite');
const patchFileName = fs.readdirSync(patchDir).find((fileName) => fileName.includes('store-database-outside-documents'));
expect(patchFileName).toBeDefined();
const patchContent = fs.readFileSync(path.join(patchDir, String(patchFileName)), 'utf8');

// The patch migrates database files by name at app startup, before any JS runs, so the
// name is hardcoded there. If the database the app opens is ever renamed, the patch (and
// this test) must be updated with it, or the migration would silently strand user data.
expect(patchContent).toContain(`@"${CONST.DEFAULT_DB_NAME}"`);

it('keeps the Onyx database name aligned with the upstream per-database migration', () => {
const sqliteProviderContent = fs.readFileSync(path.resolve(__dirname, '../../node_modules/react-native-onyx/dist/storage/providers/SQLiteProvider.js'), 'utf8');
expect(sqliteProviderContent).toContain(`'${CONST.DEFAULT_DB_NAME}'`);

// NitroSQLite now migrates each database when it opens, using the caller's database name.
const nitroSQLiteContent = fs.readFileSync(path.resolve(__dirname, '../../node_modules/react-native-nitro-sqlite/cpp/hybridObjects/HybridNitroSQLite.cpp'), 'utf8');
expect(nitroSQLiteContent).toContain('return migrateDatabase(dbName,');

@ikevin127 ikevin127 Sep 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 tests/unit/MoveFilesOutOfDocumentsTest.ts:174

I confirmed the string is present in 9.8.1 (cpp/hybridObjects/HybridNitroSQLite.cpp:86), so it passes today. Two problems remain. It is a raw substring match on upstream C++, so any reformat upstream breaks our build for no real reason. And it does not assert what the it() title claims, there is no longer anything tying CONST.DEFAULT_DB_NAME to the migration.

Meanwhile the thing that can actually regress silently is unguarded. ios/OnLoad.mm falls back without failing:

if (databaseLocation != nil && ![databaseLocation isEqualToString:@"Documents"]) {
  NSLog(@"Invalid RNNitroSQLite_DatabaseLocation value provided (%@). ... Falling back to \"Documents\".", databaseLocation);
}

A typo, or someone dropping the key in a future Info.plist edit, puts OnyxDB straight back into the user-visible Documents folder, which is the bug #96531 fixed, and nothing in CI notices. Swap the C++ grep for something that guards what we own:

it('keeps the iOS database location opted into Application Support', () => {
    const infoPlist = fs.readFileSync(path.resolve(__dirname, '../../ios/NewExpensify/Info.plist'), 'utf8');

    // NitroSQLite silently falls back to the user-visible Documents directory when this key is
    // missing or misspelled, which would put OnyxDB back in the iOS Files app.
    expect(infoPlist).toMatch(/<key>RNNitroSQLite_DatabaseLocation<\/key>\s*<string>ApplicationSupport<\/string>/);
});

});
});
Loading