Skip to content
This repository has been archived by the owner on Oct 29, 2024. It is now read-only.

Using AddComponent<Page>(parent: this) fails on iOS #138

Closed
lachlanwgordon opened this issue Jul 20, 2020 · 2 comments · Fixed by #142
Closed

Using AddComponent<Page>(parent: this) fails on iOS #138

lachlanwgordon opened this issue Jul 20, 2020 · 2 comments · Fixed by #142
Milestone

Comments

@lachlanwgordon
Copy link
Contributor

I'm trying to use Shell, it works great on Android but I can't get it working with iOS.

When I try to run any app that sets the MainPage by adding a component to the app, instead of explicitly setting main page, I get an Objective-C exception thrown. Name: NSInternalInconsistencyException Reason: Application windows are expected to have a root view controller at the end of application launch. I've seen these before when working with Xamarin Forms and it usually means you forgot to set the main page, or something went wrong before you get there.

I've also tried setting the main page to something else first, then adding my shell to app, but then I get the error Application already has MainPage set.

Reading through the comments on #41, it looks like this may be related to the GenericHost not working on iOS/UWP, but it presents very differently to the issue's headline so I thought another issue made sense.

Repro steps

  1. Clone repo from source with latest master
  2. Deploy Xaminals to iOS device or emulator
  3. App crashes
@lachlanwgordon
Copy link
Contributor Author

I've found the cause of this, and I've found a fix that works, but it's not ideal. I'll open a PR that fixes it, but hopefully we can find a way to do this properly.

The issue comes from the fire and forget at line 42 of MobileBlazorBindingsHostExtensions

// TODO: This call is an async call, but is called as "fire-and-forget," which is not ideal.
// We need to figure out how to get Xamarin.Forms to run this startup code asynchronously, which
// is how this method should be called.
renderer.AddComponent<TComponent>(CreateHandler(parent, renderer)).ConfigureAwait(false);

Which is called from the AppConstructor, which is called by FinishedLaunching() in the AppDelegate. If you don't set a MainPage by the time FinishedLaunching returns(or if it takes longer than 17 seconds), iOS will assume your app is bad and kill it.

The important line of code that never gets called is line 42 of MobileBlazorBindingsElementManager

parentAsApp.MainPage = childControlAsPage;

Work around

If we set a MainPage in the AppConstructor, we can take as long as we like to replace it with the real MainPage. There is currently a check to make sure that the MainPage isn't already set that throws an exception.

if (parentAsApp.MainPage != null)
{
                    throw new InvalidOperationException($"Application already has MainPage set; cannot set {parentAsApp.GetType().FullName}'s MainPage to {childHandler.ElementControl.GetType().FullName}");
 }

If we get rid of that, everything will be fine. I would be in favour of removing that check anyway, as I sometimes(but not often) want to change my main page, and I should be able to do this using the Host.

Setting a MainPage only to replace it instantly is a bit of a hack though.

I have tested this option and it works.

Async Attempts

I tried a few different dodgy ways to get this going with Task.Run(), while loops etc. but none of them worked. When it get's to Dispatcher.InvokeAsync(), the code inside won't run in any situation I've tried other than the fire-and-forget you've got.

I thought I'd be able to wrap the whole call in a Task.Run().Wait(), which I was hesitant to do. but when I tried it it doesn't work anyway. It seems if the Dispatcher.InvokeAsync() is inside a Task.Run() the code inside doesn't run at all, I don't know anything about the Dispatcher but this seems strange to me.

Ideally we'd be starting from an asynchronous entry point, but that could cause all sorts of issues I expect and I don't see the Xamarin.iOS opening that up any time soon.

lachlanwgordon added a commit to lachlanwgordon/MobileBlazorBindings that referenced this issue Jul 21, 2020
…eplacing MainPage

This fixes issue dotnet#138, but is a bit of a workaround rather than a proper fix, but adding this now doesn't cause any real harm or prevent more work on it later.

I've put more details about why I couldn't fix the async call in the issue comments.

# Changes
Added a placeholder page in the App constructor so that there is a root view strain away.

Updated MobileBlazorBindingsElementManager to allow MainPages being swapped at run time, there was previously a check to make sure MainPage wasn't already set, which I don't think is necessary.

Changes in MobileBlazorBindingsElementManager  look a lot more complex than they are because indentation changed when I remove the if check.
@Eilon Eilon added this to the 0.5-preview milestone Jul 21, 2020
Eilon pushed a commit that referenced this issue Jul 21, 2020
…eplacing MainPage

This fixes issue #138, but is a bit of a workaround rather than a proper fix, but adding this now doesn't cause any real harm or prevent more work on it later.

I've put more details about why I couldn't fix the async call in the issue comments.

# Changes
Added a placeholder page in the App constructor so that there is a root view strain away.

Updated MobileBlazorBindingsElementManager to allow MainPages being swapped at run time, there was previously a check to make sure MainPage wasn't already set, which I don't think is necessary.

Changes in MobileBlazorBindingsElementManager  look a lot more complex than they are because indentation changed when I remove the if check.
@Eilon
Copy link
Member

Eilon commented Jul 21, 2020

Agree with the various points made here. Async is the ultimate goal, but the workaround in your PR #142 is a perfectly reasonable workaround for the time being. Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants