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

Calling async method on ViewModel.Initialize never ends and InitializeTask properties never got updated #2829

Closed
6 tasks
vackup opened this issue Apr 24, 2018 · 8 comments

Comments

@vackup
Copy link

vackup commented Apr 24, 2018

Steps to reproduce 📜

  1. Create a ViewModel and override Initialize (this only happens if the first ViewModel is calling async code in Initialize method)

  2. Call an async / await method (eg: refit service call)

public override async Task Initialize()
        {
            await base.Initialize();

            var response = await this.Service.GetDashboardValuesAsync();
        }
  1. The device screen is black and never continue to the next line of code (eg: converting result to other model). InitializeTask status is always WaitingForActivation

Expected behavior 🤔

The execution flow should continue after awaited method complete its execution. This is working on MVVMCross 5.7

Actual behavior 🐛

The device screen is black and never continue to the next line of code (eg: converting result to other model)

Configuration 🔧

VS 2017 15.6.6

Version: MvvmCross 6 using Xamarin.Forms 2.5.1.444934

Platform:

  • 📱 iOS
  • 🤖 Android
  • 🏁 WPF
  • 🌎 UWP
  • 🍎 MacOS
  • 📺 tvOS
  • [ x] 🐒 Xamarin.Forms
@nickrandolph
Copy link
Contributor

A couple of things:

  • For Xamarin Forms, please state which runtime platform as well (unless it's the same issue on all platforms in which case please state this)
  • Please don't do long running activities in the Initialize method on the first view - this will cause problems on various devices and is not good practice (that's why it breaks in v6)
  • If you must do long running actions, please look at https://nicksnettravels.builttoroam.com/post/2018/04/19/MvvmCross-Initialize-method-on-the-first-view-model.aspx for a suggestion on how to work around the issue. It shows it for Android, but the same basic concept workf for all platforms

@ernestoyaquello
Copy link
Contributor

ernestoyaquello commented Apr 24, 2018

Why isn't the workaround implemented by default in MvvmCross? Not being able to do long asynchronous operations on the Initialize() method of the first viewmodel is very inconvenient, and it doesn't seem to make sense that long running operations would work in all viewmodels except in the first one. Besides, this implementation is in contradiction with the official documentation:

Initialize: All heavy work should be run here. This method returns a Task, which means you can mark it as async and use await safely.

@nickrandolph
Copy link
Contributor

@ernestoyaquello See the linked PR for what I'm thinking for v6.1. However, please note that you should really make use of a splash screen if you're going to be doing heavy lifting prior to the main view of your application.
The reason for the change in v6 is that there were a lot of issues due to the non-blocking nature of the Start call in MvxAppStart - doing long running activities were causing the app to not show the first view in time (particularly an issue with XF) and the app crashing or being terminated by the OS.
I'm closing this issue as the behaviour you're seeing is by design

@vackup
Copy link
Author

vackup commented Apr 25, 2018

And what about iOs? how do you suggest to use splash screen on iOs? Do you mean the basic Launch Screen or a dump View / Page that its viewModel navigates to the real first viewModel?

To avoid the launch time limitation on iOS I used to use a trick i've learnt from an old Xamarin developed app, on appDelegate , method FinishedLaunching just:

// start updating all data in the background
// by calling this asynchronously, we must check to see if it's finished
// everytime we want to use/display data.
new Thread(new ThreadStart(() =>
{
..
heavy work
...
})).Start();

Thanks for all your time and info! i really appreciate it! and thanks for your awesome work!

@ernestoyaquello
Copy link
Contributor

ernestoyaquello commented Apr 25, 2018

@nickrandolph, I understand that extremely heavy loading in the first viewmodel can cause problems, but, in my opinion, a mere warning about it in the documentation would have been better than not allowing data loading at all - speaking of which, neither this new synchronous implementation nor the workarounds to make it asynchronous again can be found in the official documentation (or at least I cannot find them?), which is odd considering that this new, app-breaking behaviour is supposed to be by design.

On the other hand, we are already using a splash screen, but I guess that you suggest we use the first viewmodel as a splash screen?

@Cheesebaron
Copy link
Member

@ernestoyaquello you expect too much from the documentation. This is an open source project made by a couple of people in the spare time. What do you prefer, good documentation or fitness of code? Pick one, not both.

@ernestoyaquello
Copy link
Contributor

ernestoyaquello commented Apr 26, 2018

@Cheesebaron I am grateful for the altruistic efforts that go into this project, but I expected to be able to load data in my first viewmodel as usual in the way that the documentation tells me to, and I think this is the right place to discuss about it. I understand that the ability to load data asynchronously in the first viewmodel might be problematic in some cases, but removing this feature completely is even more problematic in my opinion (plenty of apps will stop working), specially when there is no documentation about it and the workaround is hidden in someone's personal blog.

@yanxiaodi
Copy link

yanxiaodi commented Jul 3, 2018

I encountered the same problem. It is reasonable to place the sync codes in the Initialize method - if it works fine as it is designed. Getting the data from REST APIs is not always a heavy task for most cases.

Update: I have solved this problem following @nickrandolph 's post. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants