mphill / kysely-expo

Expo support for Kysely
MIT License
28 stars 2 forks source link

Stable Release Timeline #1

Closed nick-cheatwood7 closed 10 months ago

nick-cheatwood7 commented 10 months ago

Hi @mphill!

Do you have a rough timeline of when you expect kysely-expo to make a stable release? I see in the README that this project is still in beta and not recommended for production use quite yet.

I have an Expo/SQLite project starting early 2024 that I think this "adapter" would be a perfect fit for. I would love to be able to use this in production. I already a huge fan of Kysely and being able to use it in an Expo project would be a literal dream come true.

Thanks!

mphill commented 10 months ago

@nick-cheatwood7

I've been using this directly in my project for about 6 months, it works great.

The expo SQL library is going through a big change, with SDK 50 being the first release with the new architecture.

Please see: https://expo.dev/changelog/2023/12-12-sdk-50-beta

I planned to bump the peer dependency to require the expo-sqlite version for SDK 50.

SDK 50 is beta now, so this should happen within the next month. I will upgrade then and release it.

Unfortunately, it will require SDK 50.

mphill commented 10 months ago

@nick-cheatwood7 Have you used SQLite in the past? There are no dates, bools, json types.

I always wanted a way to transparently support these so developers did not have to constantly manage conversions.

Is this something you are interested in?

nick-cheatwood7 commented 10 months ago

@nick-cheatwood7 Have you used SQLite in the past? There are no dates, bools, json types.

I always wanted a way to transparently support these so developers did not have to constantly manage conversions.

Is this something you are interested in?

I have used it in the past and yes, a way to transparently convert between types would be awesome! Are you thinking that could be included in the kysely-expo package?

mphill commented 10 months ago

@nick-cheatwood7 I did some work on this yesterday.

There are two primary methods to accomplish this. Let's assume you want to store a boolean. There is no boolean type, you either store 1/0 or true/false as strings - both valid in SQLite.

The code can examine each element when you retrieve data from the database. If it's a 1 or 0 or even a "true" or "false" (this is valid in SQLite) these could be assumed to be boolean and the data can be transformed to real types, in this case a boolean.

It's not safe to make this assumption with 1 and 0, so it would force this library to store booleans as TEXT types be "true" or "false". Type affinity in SQLite is interesting, to say the least.

Another method would be to use column naming conventions. Any column that matches a naming convention would automatically be converted. I.e. is_active or is_available the is_ prefix would tell the library these are booleans.

This is the last item to figure out before releasing so I would appreciate any feedback you may have. Thanks.

All of the internals of the library are now upgraded to use Expo SDK 50.

nick-cheatwood7 commented 10 months ago

@mphill

I'm so stoked to hear about the upgrade to Expo 50 SDK!

I definitely understand the nuances with type conversions with SQLite. While it does sound like an interesting approach, I think the column naming convention you discussed might be too opinionated. I may be missing the point here, but since this is such a complex problem and really the last hurdle before this project can be officially released, maybe the type conversions are best left up to the end-developer?

In other words, kysely-expo should only support the same types supported by SQLite and optionally provide some utility functions like numberToBoolean() or textToBoolean() to convert to JS types, either via an import or some documentation, if the developer needs it.

I'm fully aware this may not be ideal, but I've worked with some databases (like FileMaker) in the past that, similar to SQLite, do not support certain types like booleans and instead force you to store data as text or number. I'd say 99% of the time, if we need to convert between types in JS, it's pretty trivial and most developers can just write reusable functions to handle those use cases.

This is just my two cents, but I think if the type conversions are the last major hurdle for this project, leave that to the discretion to the end-developer. That way, this package can remain lightweight and do fewer things, but do them well.

I'm open to any and all feedback. I really appreciate you welcoming ideas and feedback for this project. I can't express how excited I am to get to use this!

mphill commented 10 months ago

I am going to scrap the naming convention approach. I originally settled on that because the performance was well understood. I agree, I think it's a bit too opinionated and possibly awkward.

But I do feel support for Date and boolean are a must. Kysely doesn't do affinity conversion back to the JS runtime which is the issue. This gets SUPER confusing because in your .ts file you have a value defined as a Date type but it's a string in the runtime and you get very anomalous results - i.e. lots of not-defined errors.

I can do this conversion transparently by analyzing the data in the result, I will make this an option, but not the default.

I think that is a win-win.

nick-cheatwood7 commented 10 months ago

@mphill

Just to confirm, you're saying that the typedefs could use Date or boolean types, but the runtime would convert those to string and that's what you're trying to prevent/mask?

If so, I think that's totally reasonable. Perhaps this is not ideal, but is it possible to verify the typedefs at runtime and throw an error if any columns are defined as Date/boolean/any other type not supported by SQLite? I've admittedly never published a Typescript npm package so I don't even know if that is feasible.

Again, totally open to suggestions here. I trust you've already thought through most of this. I know Prisma sort of solved this issue internally with their SQLite adapter. I know Prisma is kind of infamous for their "black magic" under-the-hood so I don't know how much you may be able to glean from that.

mphill commented 10 months ago

Just to confirm, you're saying that the typedefs could use Date or boolean types, but the runtime would convert those to string and that's what you're trying to prevent/mask?

Yes, if would use Date and boolean the runtime for Date will be a string, and the boolean can be a 1 or "true" etc. I want to abstract that away.

There is an example app in this repo that runs the library on Expo, the README has instructions. If you can think of any good things to test, I will add them.

Thanks!

abdelhameedhamdy commented 10 months ago

Hey @mphill,

Do you have plans to support expo-sqlite/next !

Thanks

mphill commented 10 months ago

Hey @mphill,

Do you have plans to support expo-sqlite/next !

Thanks

That is the plan. I attempted a few weeks back but it wasn’t ready. Using sqlite/next will allow STRICT mode on Android.

mphill commented 10 months ago

@abdelhameedhamdy I upgraded to expo-sqlite/nextand updated the documents.

With the limited tests I have, they all passed. I am working to add more use cases.

abdelhameedhamdy commented 10 months ago

@mphill Awesome, that is a good news.

mphill commented 10 months ago

@nick-cheatwood7 Expo is releasing SDK 50 on 1/31/2024.

This library can now be installed with npm, yarn, etc. I will keep adding tests and minor tweaks, the API should not change now.

nick-cheatwood7 commented 10 months ago

@nick-cheatwood7 Expo is releasing SDK 50 on 1/31/2024.

This library can now be installed with npm, yarn, etc. I will keep adding tests and minor tweaks, the API should not change now.

This is fantastic news!

Thank you @mphill 🎉