decouple storage from activity lifecycle #62
Conversation
…ayload persist scenario we need to have storage package decoupled from activity
|
Can you apply the change to both android (pure-java and native) packages? Thanks. And also please create an issue for this so I can link it up to this change. |
|
done, |
|
So if we implement it your way, we will introduce a backward incompatibility and everyone will have to make code adjustments. Instead of changing the constructor, why don't you add a new one that only takes react context without Activity. Then in original constructor everything works as Activity is assigned to local var and in the new constructor you get the Context from React Context. Let me know once you make the adjustment and I will review the code... |
|
|
||
| public SQLitePlugin(ReactApplicationContext reactContext, Activity activity) { | ||
| public SQLitePlugin(ReactApplicationContext reactContext) { |
There was a problem hiding this comment.
As I mentioned, this create unnecessary backward incompatibility. Instead of removing original constructor, I think it will be more appropriate to add a new constructor taking only reactContext. This way both modes of creation are supported.
There was a problem hiding this comment.
no incompatibility as everyone would use Package, there is no need to create Module directly, right?
There was a problem hiding this comment.
Yes, since we have these two constructors at the Package level it should be all right.
|
I've added package constructors with activity as you suggested, but marked them as deprecated. |
to allow background data processing in particular push notification payload persist scenario we need to have storage package decoupled from activity
DB close is not required on android.
All big applications what exist on the market don't use close function as there is no appropriate place to close DB connection in app lifecycle as you are usually want to have it shared between different activities.
By using Activity callbacks you making your application work unpredictable and prohibit background usage of your package.
We are recently moved JS engine initialization to the application level to be able to process push notifications in background correctly.
You could check stack overflow and any other resources regarding to this call - it not need to be called.