Skip to content
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.

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

feat(readme): nuxt3 kysely example #484

Merged

Conversation

DawidWetzler
Copy link
Contributor

No description provided.

@jepiqueau jepiqueau merged commit 9d77285 into capacitor-community:master Nov 4, 2023
@DawidWetzler DawidWetzler deleted the add-nuxt3-kysely-example branch November 4, 2023 07:41
@jepiqueau
Copy link
Collaborator

@DawidWetzler Thanks for your contribution. i have few questions or remarks.

  • is this working only for Web or also for Native App

  • it is a bit confusing to have call this capacitor-sqlite-kyselyas it is not a capacitor plugin so this is confusing. it is more a driver or wrapper. Would be better to rename it kysely-capacitor-sqlite-driver

  • this.#sqlite.saveToStore(this.#config.name) is only valid for Web platform. You may add @capacitor/core as dependency,import { Capacitor } from '@capacitor/core'; and get the platform const platform = Capacitor.getPlatform();doing so you can test for the platform and not throw an error in native

  • for the beginTransaction, commitTransaction, rollbackTransaction method you must use the new db.beginTransaction, ... of the @capacitor-community-sqlite plugin

Apart from this, i am sure that your contribution will be greatly appreciated by the community, thanks for this

@DawidWetzler
Copy link
Contributor Author

Hello there, thanks for your feedback.

  1. It should work for all the platforms this plugin supports.

  2. I've though about calling it kysely-capacitor-sqlite but also had questions like following in my head

Q: How would I look after a extension of capacitor sqlite?
A: capacitor sqlite querybuilder
A: capacitor sqlite with kysely
A: kysely capacitor sqlite
R: capacitor-sqlite-kysely

Q: What's actually using what?
A: Kysely uses capacitor sqlite to make it work within capacitor projects.
R: kysely-capacitor-sqlite

Q: This is extension of whose functionalities?
A: Capacitor Sqlite
R: capacitor-sqlite-kysely

Q: Can I still use capacitor-sqlite outside kysely?
A: Definitely. Also shown in the example.
R: capacitor-sqlite-kysely

and for the reasons I decided to go with capacitor-sqlite-kysely but if you think this might be confusing I'm open to republish this as kysely-capacitor-sqlite or kysely-driver-capacitor-sqlite as I haven't done a pull request on the kysely repository itself for now.

  1. That's true. I could've done a more proper handling of this than just try catching it. I'll note this down and look into this in the next few days.

  2. Thanks for catching this!

@jepiqueau
Copy link
Collaborator

@DawidWetzler may be you should open a PR to kysely as it is a driver for kysely this is how it was made for typeOrm. Try to clarify with them if you can do it that way.

@DawidWetzler
Copy link
Contributor Author

Hey @jepiqueau, the issues you've found out are now fixed and the docs also now include the driver.

@jepiqueau
Copy link
Collaborator

@DawidWetzler can you share this as the link given earlier does not show it or has not been updated

@DawidWetzler
Copy link
Contributor Author

@jepiqueau
Copy link
Collaborator

@DawidWetzler i was looking on npm for kysely-capacitor-sqlite or even better kysely-driver-capacitor-sqlite and did not find it it is still called capacitor-sqlite-kysely which is not a correct naming according to the Capacitor team as it is not a capacitor plugin so you must rename it on npm. For the rest i saw what you did which is OK

@DawidWetzler
Copy link
Contributor Author

Could you please provide me the source for this? If there's this written somewhere I'd like to include this in the current repo as I have to admit I will be really unhappy with this because of:

  • signature within code won't match anymore
  • archiving repository current repository
  • creating another repository reflecting this name change
  • changing package within example
  • providing another pull request to kysely just to change the name

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants