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

Refactor connection to it's own Host Object #77

Closed
wants to merge 15 commits into from

Conversation

ospfranco
Copy link
Contributor

No description provided.

Copy link
Contributor Author

@ospfranco ospfranco left a comment

Choose a reason for hiding this comment

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

Nice work! You are on the right path @lcjuanpa. You probably already modified the cmakelists.txt on your own machine, but please also put it in this branch so CI can compile your code automatically.


namespace opsqlite {

class JSI_EXPORT OpenSqliteHo : public facebook::jsi::HostObject {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rename to DbHostObject

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please use the namespace jsi = facebook::jsi to avoid long lines


jsi::Function OpenSqliteHo::open(jsi::Runtime &rt,
const std::string &basePath) {
return HOSTFN("open", 3) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In case you still need some help: You are kinda on the right path, but this is not quite correct. You need the get, set and getPropertyNames function for the HostObject to work. If you want to declare further internal functions, there is no need to return a HOSTFN. You can declare the HOSTFNs on the header file as std::function though, that would be a cool optimization actually.

return location;
}

string OpenSqliteHo::getEncryptionKey(jsi::Runtime &rt,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you really want to go this way, I would suggest to make this generic, being able to retrieve any key from the object. Put it in utils.h though.

@lcjuanpa
Copy link

lcjuanpa commented Apr 4, 2024

Suggested changes applied on new commit

@lcjuanpa lcjuanpa closed this Apr 4, 2024
@ospfranco
Copy link
Contributor Author

Don't close this PR, we will use it to merge it to main

@ospfranco ospfranco reopened this Apr 4, 2024
@ospfranco ospfranco closed this Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants