Repository navigation
feat!: migrate to Nitro V1 - #453
chrispader wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
mrousavy
left a comment
mrousavy
left a comment
There was a problem hiding this comment.
Nice stuff so far - did you make use of the new types already? Like AnyJSON?
Requesting changes because babel definitely has to work somehow.
| export declare class NitroSQLite extends HybridObject { | ||
| /** Create a root with its own default and independent connections. */ | ||
| constructor() | ||
|
|
||
| /** Open a name-based default connection, creating the database unless `readOnly` is true. | ||
| * @param dbName Database file name and default connection key. | ||
| * @param location Directory relative to the platform database directory. | ||
| * @param readOnly Open an existing database without write access. | ||
| * @param encryptionKey Optional key for a licensed SEE build. Without SEE, a | ||
| * supplied key throws `EncryptionNotEnabled`. An invalid or wrong key or an | ||
| * existing plaintext database throws `DatabaseCannotBeDecrypted`. | ||
| */ | ||
| open( | ||
| dbName: string, | ||
| location?: string, | ||
| readOnly?: boolean, | ||
| encryptionKey?: string, | ||
| ): void |
There was a problem hiding this comment.
I see you pretty much just migrated this 1:1 to how it was before, but Nitro V1 unlocks some new patterns that make development much more elegant.
For example, I don't see why the NitroSQLite Hybrid Object has to be an instance object (aka one with a constructor()), why isn't it simply a static one where e.g. open(..) would be static open(..)?
There was a problem hiding this comment.
Chris's friendly neighborhood AI agent here. 🤖
It's an instance on purpose. Each NitroSQLite instance owns its connection registry and closes those connections in its destructor, so open databases live exactly as long as the JS runtime that opened them. With static state they would survive a reload, and reopening a database by name would then throw DatabaseAlreadyOpen. The constructor now also takes the directories from the new PlatformContext (#462). If you'd still rather have statics, we can change it.
| /** Android startup hook that configures the native database directories. */ | ||
| @Android({ | ||
| language: 'kotlin', | ||
| implementation: { className: 'HybridNitroSQLiteOnLoad' }, | ||
| }) | ||
| export declare class NitroSQLiteOnLoad extends HybridObject { | ||
| /** Pass the app's files and cache directories from the application context to SQLite. */ | ||
| static init(): void | ||
| } |
There was a problem hiding this comment.
I'm wondering if that should be moved into NitroSQLite too? Or not called *OnLoad but instead something like PlatformDirectoryHelper or something idk? Then that returns a directory or whatever and can be passed into NitroSQLite? No idea what it does, just some thoughts.
I think react-native-mmkv has a similar pattern with PlatformObject or whatever it is called, this creates the cache directory on Kotlin/Swift while the other object was in C++.
There was a problem hiding this comment.
Chris's AI wingmate checking in. 🤖
Good idea. NitroSQLiteOnLoad only told C++ where to store databases on Android, and iOS did the same through an Objective-C +load. #462 replaces both with a PlatformContext hybrid object, implemented in Kotlin on Android and Objective-C++ on iOS, that returns the database, temporary, and legacy database directories. JavaScript passes them into the NitroSQLite constructor, so the JNI setter and the +load hook are gone.
| import { NitroModules } from 'react-native-nitro-modules' | ||
| import type { NitroSQLite as NitroSQLiteSpec } from './specs/NitroSQLite.nitro' | ||
| import type { NitroSQLite } from './specs/NitroSQLite.nitro' | ||
|
|
||
| export const HybridNitroSQLite = | ||
| NitroModules.createHybridObject<NitroSQLiteSpec>('NitroSQLite') | ||
| // Look the class up by name so Metro never loads the spec, whose decorators need a Babel parser plugin. | ||
| const NitroSQLiteClass = NitroModules.getHybridClass<typeof NitroSQLite>( | ||
| 'rnnitrosqlite.NitroSQLite', | ||
| ) | ||
|
|
||
| export const HybridNitroSQLite = new NitroSQLiteClass() |
There was a problem hiding this comment.
That's not good - you should get the babel to work. I think its just a simple boolean switch to allow .cjs exports in babel. Nitro generates those .cjs classes for you which essentially do exactly what you just wrote here.
There was a problem hiding this comment.
Chris's AI sidekick here. 🤖
Agreed, and it's fixed now. #453 imports the classes from the specs, so src/nitro.ts uses the generated #nitro/* modules instead of its own getHybridClass lookup. The other half is in #457. React Native's default Babel preset can't parse the spec decorators, and our react-native field pointed at src, so apps would have compiled the specs themselves. The entry points now point at the built output, and bob parses the decorators with a Babel config for each target.
| "imports": { | ||
| "#nitro/*": "./nitrogen/generated/shared/javascript/*.cjs" | ||
| }, |
a405776 to
c02fcda
Compare
|
Chris's AI understudy, reporting for duty. 🤖
I also split this PR into a stack so each part can be reviewed on its own:
The stack builds on #455, which upgrades the example to React Native 0.88. |
a405776 to
c02fcda
Compare
597fff3 to
0a20e65
Compare
Rewrite the Nitro specs as decorated classes, regenerate the bindings, and port the C++ hybrid objects to V1's value-type Promise, ArrayBuffer and Callback APIs. JavaScript imports the hybrid classes from the specs, which re-export the runtime modules that nitrogen generates under Nitro V1 doesn't support CocoaPods, so the iOS, macOS and native lockfile jobs are paused. The Nitro 0.37 patch moves into the macOS example because the root install can't apply it to Nitro V1. BREAKING CHANGE: Requires React Native 0.88 and Nitro V1. Co-Authored-By: Claude Opus 5.5 <[email protected]> Co-authored-by: Codex <[email protected]>
The podspec loads CocoaPods files that nitrogen V1 no longer generates, so the test can't run until iOS builds with Swift Package Manager. Co-Authored-By: Claude Opus 5.5 <[email protected]>
0a20e65 to
3bee6c9
Compare
Nitro SQLite still uses Nitro 0.37, so apps that move to the Nitro V1 alpha can't build it. V1 declares specs as decorated classes, generates the JavaScript runtime modules, and requires React Native 0.88. This PR ports the specs, the C++ hybrid objects, and the Android code to Nitro
1.0.0-alpha.nightly.86.g15e19db42041. The JavaScript code imports the hybrid classes from the specs, which re-export the runtime modules that nitrogen generates under#nitro/*.A few changes need a closer look:
NitroSQLiteno longer typesclose,drop, and the other native methods that its object spread never copied at runtime. They stay available onNitroSQLite.native.🤖 Generated with Claude Code