sp-react-native-in-app-updates icon indicating copy to clipboard operation
sp-react-native-in-app-updates copied to clipboard

Update README(added an example)

Open nirajniroula opened this issue 4 years ago • 12 comments

Added a basic working code for both types of update implementation.

nirajniroula avatar Oct 26 '21 16:10 nirajniroula

Thanks for the PR, but I believe this needs to be addressed properly by fixing the actual example files in the project, instead of adding more instructions on the readme.

If you want to you can add that as a second example, and point to it from the readme.

Can you also please remove the react-native-snackbar references and code?

I'd be grateful!

Thanks

SudoPlz avatar Oct 26 '21 17:10 SudoPlz

Sure, I can add a second example in a separate file, like the current one, and point to it from the README.

But regarding react-native-snackbar usages, I just wanted to implement the flexible updates as illustrated in the android docs believing it to be one of the right approaches. If you want me to remove that, I can do that too but I think that should not be an issue, at least not for an example.

Thank you for the quick response.

nirajniroula avatar Oct 27 '21 06:10 nirajniroula

With snack bar, the update flow somewhat becomes like this:

  1. a dialog appears saying "Xyz recommends that you update to the latest version. You can keep using this app while downloading the update."

  2. user presses UPDATE button, download starts in the background, the user continues using the app

  3. snack bar appears after the update is downloaded with an INSTALL button (user can complete what he is about to do in the app, and install the new update afterwards i.e. the user won't be interrupted by the sudden installation process )

But again with this approach, there is one more thing that should be taken care of. What if a user downloads a new update but never installs it! There should be a way to get the preserved update state(DOWNLOADED in this case) from the native side(methods) so that on every launch, the user gets reminded about the update that was downloaded earlier and if he/she wants to installs it, via snack bar. I remember handling this with a listener while integrating it in a native android app. I shall look into it, if I get some free time. But for now, in this example, I have used async-storage to store the update state so that if a user has already downloaded but not installed the new update, he/she gets notified about the downloaded update on every app launch.

Just trying to clarify the example. Hope I made it a little clearer.

nirajniroula avatar Oct 27 '21 11:10 nirajniroula

Cool, let's keep the snackbar, I think it's a-ok. About FLEXIBLE builds I was under the impression that's handled automatically when startUpdate... fires up again for the second time (after a binary has already been downloaded) but feel free to double check since, I admit we're not using that and I haven't tested that scenario thoroughly.

SudoPlz avatar Oct 27 '21 13:10 SudoPlz

It won't reach up to startUpdate for the second time.


 useEffect(() => {
      inAppUpdates.addStatusUpdateListener(status => onStatusUpdate(status));
  }, []);

...
...

   inAppUpdates
      .checkNeedsUpdate()
      .then(result => {
        if (result.shouldUpdate) {
          let updateOptions = {...};

          inAppUpdates.startUpdate(updateOptions);
        }
      })
      .catch(err => console.log("IAU Error: ", err));

Looks like result.shouldUpdate is false for the second time. I'm not sure about it but I can confirm that, in the code block above, the onStatusUpdate(status) method didn't execute the second time.

nirajniroula avatar Oct 27 '21 15:10 nirajniroula

I think right way to handle previously downloaded builds it to check for the UpdateAvailability.DEVELOPER_TRIGGERED_UPDATE_IN_PROGRESS state in updateAvailability. This should be more reliable than using the AsyncStorage method.

I implemented something like this in a fork: https://github.com/Hilokal/sp-react-native-in-app-updates/commit/5dc74713c5988f204dca02b2c1c7c5bf5ea66d91

And then in the client I do something like this:

const result = await inAppUpdates.checkNeedsUpdate();

if (result.other.installStatus === IAUInstallStatus.DOWNLOADED) {
  inAppUpdates.installUpdate();
} else {
  inAppUpdates.startUpdate({ updateType: IAUUpdateKind.FLEXIBLE });
}

jbaudanza avatar Mar 11 '22 01:03 jbaudanza

@jbaudanza can you open a PR with your addition? Or @nirajniroula do you think you can add it to this PR?

SudoPlz avatar Mar 11 '22 12:03 SudoPlz

Since this PR was only about adding an example (to achieve a better UX) in README, I think it would be better if @jbaudanza, you could open a separate PR from https://github.com/Hilokal/sp-react-native-in-app-updates/commit/5dc74713c5988f204dca02b2c1c7c5bf5ea66d91. And maybe for this PR, as suggested by @SudoPlz earlier, I can add the second example in a separate file, like the current one, and point to it from the README. I'm not being able to get back to it due to my occupation mainly in web development lately. I will do it whenever I get a chance to test the example thoroughly once again and maybe I shall also remove the AsyncStorage part from the example in the next push to this PR if the above fork gets merged to the master by then.

nirajniroula avatar Mar 11 '22 15:03 nirajniroula

I actually went in a different direction in my branch. I removed all the dependencies and everything that wasn't a direct binding to the InAppUpdates API. This made it much easier for me to integrate into my app.

I think the Apple App store API is different enough that trying to make a common API for both ends up being awkward.

Also siren brings in a lot of dependencies (apisauce, axios) for what is just a simple JSON fetch. I removed it and replaced with with a simple fetch() call and Alert directly in my app.

This is everything I removed: https://github.com/Hilokal/sp-react-native-in-app-updates/compare/master...hilokal

If you're interested in going in this direction, I can prepare a PR.

Otherwise, if you only want the DEVELOPER_TRIGGERED_UPDATE_IN_PROGRESS, we should discuss what the API would look like. Maybe adding a shouldInstall boolean in addition to the shouldUpdate boolean would work. But, personally, I'd be in favor of just trimming down everything and letting the consumer of this module handle all that logic directly.

jbaudanza avatar Mar 11 '22 17:03 jbaudanza

@SudoPlz Would there still be interest into implementing @jbaudanza 's solution?

GoldenSoju avatar May 08 '24 01:05 GoldenSoju

@GoldenSoju I would love to find out what the community thinks about that.

Both make sense to me, I agree that siren does add some unnecessary dependencies but on the other hand it's a set and forget thing where you install once, and it should work on both platforms.

I think in an ideal world we would strip down this library so that

  • it doesn't include any other dependencies like siren, but then
  • add a module on top of it so that it works for iOS too (and then it would include siren too) but I'm
  • not sure how to structure that and make it easy enough for users while at the same time
  • make it obvious that this will be a breaking change if they choose to migrate to that new version which separates concerns

SudoPlz avatar May 09 '24 15:05 SudoPlz

@GoldenSoju @SudoPlz In my opinion, the user flows between Android In-App-Updates and iOS app store checks are so different, that it's not worth trying to unify into a single API. The react-native module should provide a clean Javascript interface to the Android In-App-Updates API, and it should be the role of the app developer to navigate the differences in the user flows in a way that makes sense to their app.

It's also been 2 years since I looked at this, so it's possible my memory is faded. But, I remember trying to make things work in siren to be more difficult than it was worth. In particular, siren has hardcoded the strategy for comparing version numbers: https://github.com/GantMan/react-native-siren/blob/1bb4945d102a2b577dfee51742dbe8d0e358c63d/index.js#L83

In our app, we ended up replacing siren with a simple fetch() call.

jbaudanza avatar May 10 '24 00:05 jbaudanza