Add C ABI target with support for iOS and Android #4774
Conversation
|
|
query-engine/connector-test-kit-rs/query-tests-setup/src/config.rs
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
query-engine/connector-test-kit-rs/query-tests-setup/src/connector_tag/sqlite.rs
Outdated
Show resolved
Hide resolved
query-engine/connector-test-kit-rs/query-tests-setup/src/connector_tag/sqlite.rs
Outdated
Show resolved
Hide resolved
| // .await | ||
| } | ||
|
|
||
| async unsafe fn apply_migrations(&self, migration_folder_path: *const c_char) -> Result<(), Error> { |
There was a problem hiding this comment.
While I understand that pulling in the entire schema engine was way too big to simply handle that, it does look like a lot of duplicated logic that we will have to maintain long-term. I don't want this to be blocking the merge of the PR, but we should def. add some TODO here to think about how to pull in bits of logic from the schema engine to handle this, without having to pull in the entire crate.
There was a problem hiding this comment.
Yup definitely, at the same time, the use case will be different. Handling errors will be done differently on thousands of devices at the same time. While on the server the person doing the migration has hands-on keyboard, on mobile this will need to have a lot more failsafe to allow apps to start in case of a failed migration.
I haven't completely thought through was this means since this is a completely new use case for Prisma.
For now I will add the TODO but it might actually make sense to keep it separated.
There was a problem hiding this comment.
Great job, this is quite a lot of work! @SevInf and I have left a bunch of comments. Don't hesitate to ask us for help if anything's unclear or you don't understand what we're asking you. There is still the matter of the tests to figure out before we merge this. Let's discuss this in Slack.
There was a problem hiding this comment.
This file is mostly a duplicate of query-engine-node-api. We now have 3 versions of the roughly the same file. We won't ask you to DRY these 3 loggers, this is mostly a note for us to remember to do it. I've created an issue here https://github.com/prisma/team-orm/issues/1054
CodSpeed Performance ReportMerging #4774 will not alter performanceComparing Summary
|
|
@ospfranco native QE tests should be fixed by now, to make driver adapters pass you need to pull in latest |
This is part of an effort to potentially bring Prisma to the mobile world via React Native and Expo.
React native can directly bind C++ functions to JS. Rust can expose functions via C ABI. Therefore I have added a new target for the query engine that exposes this functionality via C-style calls.
This target also aiming for Android and iOS has added some conditional compilation to other parts of the engine. So it is not an isolated change, therefore it's important to do a thorough review.
Some merge artifacts might have remained, so let me know if you find anything.