chronogolf / nativescript-store-update

Apache License 2.0
19 stars 13 forks source link

Init conflict with firebase iOS10.3 #21

Closed sitefinitysteve closed 6 years ago

sitefinitysteve commented 6 years ago

Here's the start of the thread in question https://github.com/EddyVerbruggen/nativescript-plugin-firebase/issues/409#issuecomment-346868997

Relevant video: https://www.screencast.com/t/sxIHnKMtH

Okay so when the init code is in the app.ts

    StoreUpdate.init({
        notifyNbDaysAfterRelease: 0,
        majorUpdateAlertType: AlertTypesConstants.FORCE,
        minorUpdateAlertType: AlertTypesConstants.FORCE,
        patchUpdateAlertType: AlertTypesConstants.FORCE,
        revisionUpdateAlertType: AlertTypesConstants.FORCE
    });

...and google-signin sends the user back into the app (watch the video, only iOS10.3) something fails or gets intercepted and it doesn't properly complete and the user is just hanging out in Safari.

If I comment out that code or move it out of app.ts it all works as expected.

sitefinitysteve commented 6 years ago

So I put the code on my landing\home screen as this

    StoreUpdate.init({
        notifyNbDaysAfterRelease: 0,
        majorUpdateAlertType: AlertTypesConstants.FORCE,
        minorUpdateAlertType: AlertTypesConstants.FORCE,
        patchUpdateAlertType: AlertTypesConstants.FORCE,
        revisionUpdateAlertType: AlertTypesConstants.FORCE
    });

    StoreUpdate.checkForUpdate();

It works, however when I navigate my app BACK to that screen, it then throws

1   0x1034fcd4b NativeScript::FFICallback<NativeScript::ObjCMethodCallback>::ffiClosureCallback(ffi_cif*, void*, void**, void*)
2   0x103ba028e ffi_closure_unix64_inner
3   0x103ba0bd2 ffi_closure_unix64
4   0x1045283c3 -[UIViewController __viewWillAppear:]
5   0x1045523c8 -[UINavigationController _startCustomTransition:]
6   0x104562967 -[UINavigationController _startDeferredTransitionIfNeeded:]
7   0x104563b41 -[UINavigationController __viewWillLayoutSubviews]
8   0x10475560c -[UILayoutContainerView layoutSubviews]
9   0x10444255b -[UIView(CALayerDelegate) layoutSublayersOfLayer:]
10  0x109c7c904 -[CALayer layoutSublayers]
11  0x109c70526 CA::Layer::layout_if_needed(CA::Transaction*)
12  0x109c703a0 CA::Layer::layout_and_display_if_needed(CA::Transaction*)
13  0x109bffe92 CA::Context::commit_transaction(CA::Transaction*)
14  0x109c2c130 CA::Transaction::commit()
15  0x104378367 _UIApplicationFlushRunLoopCATransactionIfTooLate
16  0x104b797a3 __handleEventQueue
17  0x106a0ec01 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__
18  0x1069f40cf __CFRunLoopDoSources0
19  0x1069f35ff __CFRunLoopRun
20  0x1069f3016 CFRunLoopRunSpecific
21  0x107ec3a24 GSEventRunModal
22  0x10437f134 UIApplicationMain
23  0x103ba0a2d ffi_call_unix64
24  0x11f0a6560
file:///app/tns_modules/nativescript-store-update/store-update.js:21:66: JS ERROR Error: NS Store Update already configured

Maybe unrelated, but wrapping it in a check prevents the crash

    if (storeInitalized == null) {
        StoreUpdate.init({
            notifyNbDaysAfterRelease: 0,
            majorUpdateAlertType: AlertTypesConstants.FORCE,
            minorUpdateAlertType: AlertTypesConstants.FORCE,
            patchUpdateAlertType: AlertTypesConstants.FORCE,
            revisionUpdateAlertType: AlertTypesConstants.FORCE
        });

        storeInitalized = true;
    }    

    StoreUpdate.checkForUpdate();
EddyVerbruggen commented 6 years ago

@sitefinitysteve The entire app delegate is overwritten by this plugin so any other plugin depending on methods fired from the appdelegate are in trouble.

I don't blame the author at all for this as I actually think this is a more generic problem with NativeScript plugins; there should be a way for them to safely register callbacks with the appdelegate so we don't need to worry about it. But that's a tough cookie to crack.

In the meanwhile, here's an idea: as the methods of this plugin actually don't have to be in the appdelegate, why not listen to the app lifecycle events instead? The application.launchEvent and application.resumeEvent seem like the perfect hooks for the current appdelegate checks.

SBats commented 6 years ago

Hi @sitefinitysteve. Thank for reporting this issue, and thanks @EddyVerbruggen for the explanation and the idea. That might actually just work with those events! We'll try to fix this as soon as we have a moment. Definitely a good teaching regarding delegates.

sitefinitysteve commented 6 years ago

@EddyVerbruggen is the master...

SBats commented 6 years ago

Should be fixed by #22 . Thx a lot @sitefinitysteve for the contribution! Please reopen the issue if needed.